MartinThoma / flake8-simplify

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

[New Rule] Replace `hasattr` if else with `getattr` when appropiate #180

Open Skylion007 opened 1 year ago

Skylion007 commented 1 year ago

Explanation

This code is more succinct, readable, and slightly more efficient. We recently opened a few PRs to add this to the PyTorch codebase. Exhibit A: https://github.com/pytorch/pytorch/pull/100360 If you are aware of a different flake8-plugin / ruleset that implements this rule, feel free to recommend it.

Example

# Bad
...  # your bad code goes here
X.Y if hasattr(X, "Y") else Z

# Good
...  # your improved code goes here
getattr(X, "Y", Z)
r-barnes commented 1 year ago

Note that this is not always more efficient!

The pattern

X.Y if hasattr(X, "Y") else expensive_function()

doesn't evaluate expensive_function(), so it is preferable to

getattr(X, "Y", expensive_function())

This is also bad in the case where we have:

X.mutually_exclusive_1 if hasattr(X, "mutually_exclusive_1") else X.mutually_exclusive_2

Granted: having the interface of a class change like this is bad coding, but it does happen.

In cases where the Z is a constant of some sort, then getattr seems like a slam-dunk for readability in my opinion.