Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EC69 : Irrelevant rule for Python code #21

Closed
Paul-Wassermann-CA opened this issue Mar 11, 2024 · 3 comments · Fixed by #22
Closed

EC69 : Irrelevant rule for Python code #21

Paul-Wassermann-CA opened this issue Mar 11, 2024 · 3 comments · Fixed by #22
Assignees
Labels
🗃️ rule rule improvment or rule development or bug

Comments

@Paul-Wassermann-CA
Copy link

Paul-Wassermann-CA commented Mar 11, 2024

Rule EC69 is irrelevant for Python. The rule seems to imply that calling a function in a for statement results in calling this function multiple times, which is not true.

The two following syntaxes are strictly equivalent regarding the number of calls to the range builtin function :

for num in range(10):
    pass

iterable = range(10)
for num in iterable:
    pass

A simple way to prove see this is using a counter :

NUM_CALLS = 0


def func_to_iterable():
    global NUM_CALLS
    NUM_CALLS += 1
    return range(10)


def supposedly_ok_function():
    iterable = func_to_iterable()
    for num in iterable:
        pass


def supposedly_bad_function():
    for num in func_to_iterable():
        pass


if __name__ == '__main__':
    supposedly_bad_function()
    supposedly_ok_function()

    print(NUM_CALLS)

This prints 2 since the function is called only twice.

Therefore, I think the rule should be dropped since it's confusing and doesn't make much sense from an eco-design perspective, at least for Python, but it should be investigated for other languages too.

@Horrih
Copy link

Horrih commented Mar 20, 2024

Agreed, I got bitten by this non-issue yesterday on a work project when activating the eco rules.

Is it a failed port of an equivalent rule in another language ?

I believe this rule should be simply removed.

@dedece35
Copy link
Member

Hi, thank you for your demonstration.
What do you think about this issue @jhertout ? are you ok to remove this rule for python ?

I'm ok to check on other language.

@dedece35 dedece35 added the 🗃️ rule rule improvment or rule development or bug label Mar 20, 2024
@jhertout
Copy link

Hello @dedece35. I perform some tests as described in this issue to verify and indeed I think we can remove this rule for Python.
May be we can check this rule in other languages too and maybe there are differences between old traditionnal for loop (for int i = 0 ; i < 10 ; i++) and foreach loop in other language where the 2 kinds of loops are possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗃️ rule rule improvment or rule development or bug
Projects
None yet
4 participants