Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid getattr(x, 'foobar') #46

Closed
vergeev opened this issue Jul 13, 2018 · 3 comments
Closed

Forbid getattr(x, 'foobar') #46

vergeev opened this issue Jul 13, 2018 · 3 comments
Labels
rule request Adding a new rule

Comments

@vergeev
Copy link
Contributor

vergeev commented Jul 13, 2018

As per the docs:

getattr(x, 'foobar') is equivalent to x.foobar

This may lead a reader to believe that getattr(obj, 'attribute') will not throw an exception.

Correct

x.foobar
getattr(x, 'foobar', None)
getattr(x, some_variable, None)

Incorrect

getattr(x, 'foobar')
@sobolevn sobolevn added the rule request Adding a new rule label Jul 13, 2018
@sobolevn
Copy link
Member

But what if we have the following function:

def fetch_property(obj, key_name):
     return getattr(obj, key_name)

And I want to throw an exception here! Because I prefer an exception over None.
You solution implies that we will have to manually raise these exceptions:

foobar = getattr(x, 'foobar', None)
if foobar is None:
    raise AttributeError()

@vergeev
Copy link
Contributor Author

vergeev commented Jul 13, 2018

Good catch! What I actually meant is prohibit the case in which the second argument is a string literal (which is always something constant).

I've updated the issue.

@sobolevn
Copy link
Member

Anyway, this does not remove my consideration about consciously rising an exception here.
I use both versions with 50/50 rate.

@vergeev I guess that you are talking about beginners, who make this error and crush their programs. But, that's not the case here.

Thanks for your suggestion, but I am going to reject it.
Please, open new ones if you have any. I appreciate your attitude towards this project.

Cheers. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule request Adding a new rule
Projects
None yet
Development

No branches or pull requests

2 participants