[Adjust Rule] SIM116 Use a dictionary lookup instead of 3+ if/elif - but not if side effects are involved

[tsx]

Desired change

• Rule(s): SIM116
• Adjustment: Don't suggest a dictionary when the value is a side-effecting function call.

Explanation

Constructing a dictionary of all values will invoke all side effects from all branches, which is not desired.

On top of that, the suggestion wrongly quotes the values.

Example

class Cat:
    weight = 10
    sound = 'purr'

def launch(launchable):
    print('{} have been launched!'.format(launchable))
    return len(launchable)

def pet(animal):
    print(animal.sound + '!')
    return animal.weight

def example(action, **kwargs):
    if action == 'launch':
        return launch('Rockets')
    elif action == 'pet':
        return pet(kwargs['animal'])
    elif action == 'idle':
        return None

# Bad suggestion:
# SIM116 Use a dictionary lookup instead of 3+ if/elif-statements: return {'launch': "launch('Rockets')", 'pet': "pet(kwargs['animal'])", 'idle': 'None'}.get(action)

def bad_suggestion(action, **kwargs):
    return {
        'launch': launch('Rockets'),
        'pet': pet(kwargs['animal']),
        'idle': None,
    }.get(action)

>>> bad_suggestion('pet', animal=Cat())
Rockets have been launched!
purr!

[ergoithz]

I don't think this is a bad suggestion, you can always use a lambda mapping to turn your O(n) if-elif-else into a O(1) operation.

def example(action, **kwargs):
    def default(**_):
        return None

    actions = {
        'launch': lambda **_: launch('Rockets'),
        'pet': lambda *, animal: pet(animal),
        'idle': lambda **_: None,
    }

    return actions.get(action, default)(**kwargs)

You can even omit the intermediate lambdas if you prepare your functions for this usage.

Alternatively, I tend to suggest a getattr approach (__dict__ lookup) for the most complex scenarios:

class case:
    @classmethod
    def handle_launch(cls, **_):
        return launch('Rockets')

    @classmethod
    def handle_pet(cls, *, animal):
        return pet(animal)

    @classmethod
    def handle_idle(cls, **_):
        return None

    @classmethod
    def handle_default(cls, **_):
        return None

    @classmethod
    def handle(cls, action, **kwargs):
        return getattr(cls, f'handle_{action}', cls.handle_default)(**kwargs)

The above are quite common patterns used on parsers.

This of course would be no longer necessary with Python 3.10 match/case (PEP634).

[MartinThoma]

I think the mentioned issue with SIM116 was fixed in the context of https://github.com/MartinThoma/flake8-simplify/issues/113 . I'll make a release with that fix today.