[Adjust Rule] SIM113 False Positive in Nested Loops about flake8-simplify HOT 7 CLOSED

Ok, true, this might be a nicer way for the simple case 😃

You might want to add this example to the main Readme so people know in one second how to fix this linting issue.

from flake8-simplify.

@GitRon Done :-) I've also linked the issues in which the rules were introduced and renamed the titles of the issues so that they are easier to find. Additionally, I've added a short description of the rule + a link to the example later in the document. I might move the complete set of rules and examples to another page in the future as I'm not a big fan of huge READMEs.

from flake8-simplify.

Good point! Thank you for raising it.

At the moment, I'm not sure how to fix it. Or if there is any way to fix it. I'm thinking about removing SIM113 completely again.

For now, you can set --ignore=SIM113 or within the setup.cfg

[flake8]
ignore=SIM113

from flake8-simplify.

Just gave your package a shot and its really nice - but I didn't get the SIM113 (which is not documented on the main page btw). Whats the reason not to use a regular var as a loop counter?

Thanks!

from flake8-simplify.

@GitRon Thank you :-)

SIM113 should bring awareness to people who are not aware of enumerate. The advantage of using enumerate (when possible) is that you don't have to increment that variable in the loop body. It is a tiny bit easier to read as the reader does not have to figure out where / when the loop variable is incremented (or if it is incremented in multiple places).

Of course, Skylion007 gave a good example with

count = 0
foos = [1, 2, 3, 4]
bars = ["A", "B", "C"]
for foo in foos:
    for bar in bars:
        count +=1

where this rule should not be triggered.

By the way, it is possible to use enumerate here as well:

count = 0
foos = [1, 2, 3, 4]
bars = ["A", "B", "C"]

from itertools import product
for count, (foo, bar) in enumerate(product(foos, bars)):
    print((count, foo, bar))

But that is harder to read, especially when more happens in the function body.

I'm currently wondering if checking for nested loops via the introduced parent attribute solves the issue.

from flake8-simplify.

Thanks for the explanation! Actually I didn't know about this as well. Looks fancy but I might argue for the simple solution to be valid as well. Linting is quite strict so IMHO I think it should only point out things, you should never do. And if you want to keep a complicated case simple, it might be valid to to for approach one, won't it?

from flake8-simplify.

It depends on the exact case. For the example given in this issue by Skylion007 , I agree that NOT using enumerate is the simpler solution.

In the following case, I want to encourage people to use enumerate:

# Bad, SIM113 should be triggered:
i = 0
for element in some_iterable:
    ...   # do something with i
    i += 1

# Good
for i, element in enumerate(some_iterable):
    ...  # do something with i

from flake8-simplify.