MartinThoma / flake8-simplify

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

[Adjust Rule] SIM108: False-positive for if-elif-else #115

Closed MartinThoma closed 2 years ago

MartinThoma commented 2 years ago

Desired change

Explanation

The shown error message is wrong

Example

        if E == 0:
            M = 3
        elif E == 1:
            M = 2
        else:
            M = 0.5

Gives this message:

./17_Bullfight/python/Bullfight.py:144:14: SIM108 Use ternary operator 'M = 2 if E == 1 else 0.5' instead of if-else-block
MartinThoma commented 2 years ago
$ astpretty --no-show-offsets /dev/stdin <<< `cat example.py`
Module(
    body=[
        If(
            test=Compare(
                left=Name(id='E', ctx=Load()),
                ops=[Eq()],
                comparators=[Constant(value=0, kind=None)],
            ),
            body=[
                Assign(
                    targets=[Name(id='M', ctx=Store())],
                    value=Constant(value=3, kind=None),
                    type_comment=None,
                ),
            ],
            orelse=[
                If(
                    test=Compare(
                        left=Name(id='E', ctx=Load()),
                        ops=[Eq()],
                        comparators=[Constant(value=1, kind=None)],
                    ),
                    body=[
                        Assign(
                            targets=[Name(id='M', ctx=Store())],
                            value=Constant(value=2, kind=None),
                            type_comment=None,
                        ),
                    ],
                    orelse=[
                        Assign(
                            targets=[Name(id='M', ctx=Store())],
                            value=Constant(value=0.5, kind=None),
                            type_comment=None,
                        ),
                    ],
                ),
            ],
        ),
    ],
    type_ignores=[],
)
MartinThoma commented 2 years ago

Uuuuh, damn: What this proposes is technically correct, but for sure harder to understand.

Original

if E == 0:
    M = 3
elif E == 1:
    M = 2
else:
    M = 0.5

Current flake8-simplify proposal

if E == 0:
    M = 3
else:
    M = 2 if E == 1 else 0.5
MartinThoma commented 2 years ago

If there were a lot more values, I would go with the dictionary pattern:

M = {0: 3, 1: 2}.get(E, 0.5)

But here, I think nothing should be changed.