MartinThoma / flake8-simplify

❄ A flake8 plugin that helps you to simplify code
MIT License
185 stars 19 forks source link

[Adjust Rule] SIM102 - comments in between if clauses #61

Open MetRonnie opened 3 years ago

MetRonnie commented 3 years ago

Desired change

Explanation

Sometimes it is clearer to split up logic into separate if conditions with comments explaining what is going on under the if conditions

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

for p in reversed(path.parents):
    ...
    if p == path.parent:
        # This is the last iteration
        if p in some_list and some_bool:
            do_something()

With the current rule, I guess you can do this:

for p in reversed(path.parents):
    ...
    if (
        p == path.parent
        # This is the last iteration
        and
        p in some_list and some_bool
    ):
        do_something()

but it is less readable in my opinion.

MartinThoma commented 2 years ago

I would try to get rid of the comment, e.g. as:

for p in reversed(path.parents):
    ...
    is_last_iteration = p == path.parent
    if is_last_iteration and p in some_list and some_bool:
        do_something()
MartinThoma commented 2 years ago

@MetRonnie Do you have an example where getting rid of the comments would not be desirable?

MetRonnie commented 2 years ago

Mainly my example in the OP. Your suggestion of creating a variable feels a bit inefficient to me, and I could imagine scenarios where a lengthier comment that couldn't be summarised with a variable (although admittedly I haven't run into any since opening the issue).