sourcery icon indicating copy to clipboard operation
sourcery copied to clipboard

hoist-if-from-if with same condition results in redundant check

Open pzelnip opened this issue 3 years ago • 2 comments

Issue description or question

If you have code like:

def find_item(needle, haystack):
    result = haystack.search(arg1=True, arg2=False, field1=needle)
    if not result:
        result = haystack.search(arg1=True, arg2=False, field2=needle)
        if not result:
            raise NotFound()

    return result

Sourcery will flag the 2nd if not result with hoist-if-from-if and indicate it should be rewritten as:

def find_item(needle, haystack):
    result = haystack.search(arg1=True, arg2=False, field1=needle)
    if not result:
        result = haystack.search(arg1=True, arg2=False, field2=needle)
    if not result:
        raise NotFound()

    return result

But this is (slightly) less efficient, if the first search call finds a match, then you'd end up evaluating if not result twice in the sourcery version, but only once in the original.

Sourcery Version

0.9.8 of the VS Code extension

Code editor or IDE name and version

VS Code Version: 1.63.2 Commit: 899d46d82c4c95423fb7e10e68eba52050e30ba3 Date: 2021-12-15T09:37:28.172Z Electron: 13.5.2 Chromium: 91.0.4472.164 Node.js: 14.16.0 V8: 9.1.269.39-electron.0 OS: Darwin x64 19.6.0

OS name and version

OSX - Catalina, 10.15.7

pzelnip avatar Jan 07 '22 18:01 pzelnip

Thanks for raising this! We will have a think about this proposal.

Hellebore avatar Jan 11 '22 14:01 Hellebore