Reputation: 2526
Why is this decorator strategy considered bad? (..or is it!?)
class User(object):
def __init__(self):
self.thing = 5
def __atomic_rate_change(fn):
def wrapper(self,*args,**kwargs):
print "start magic"
self.thing += 1
fn(self,*args,**kwargs)
print "end magic"
return wrapper
@__atomic_rate_change
def foo(self,add):
print self.__atomic_rate_change # <bound method User.__atomic_rate_change of <__main__.User object at 0x6ffffe1ef50>>
self.thing += add
print "normal call {0}".format(self.thing)
test = User()
test.foo(1)
This works. But, according to resource below, it's bad practice. Reasons would be that:
[...] there is major flaw in this approach: atomic_rating_change becomes an instance method of the User class. That doesn’t make any sense. More than this, it doesn’t even work as a method: if you call it, the decorated parameter will be used as self.
https://medium.com/@vadimpushtaev/decorator-inside-python-class-1e74d23107f6
I don't understand why it's a problem/wrong/bad that atomic_rate_change is a instance method. I'm only intending the decorator to be used within the class. Perhaps in this case it's okay?
Upvotes: 3
Views: 85
Reputation: 2947
Stylistically, placing function definitions into the class definition which are not methods are kind of out of place (imho it can even be unpythonic). Flat is better than nested, so it is probably better to declare the function outside of the class. This way when the reader is looking at your class, there won't be the confusion of why there is a method which does not take self
as an argument (because the function is declared like a method when it is merely a decorator, though this is slightly different if the function is a @staticmethod
).
If you're worried about it being used outside of the class, prefix it with an _
and then from my_package import *
won't import it. It can still be used in that module, but it won't be used outside unless explicitly imported.
Practically, the author is referring to the occasional odd behavior of scoping (similar to the debates in Javascript on whether to use function() { ...
or () => { ...
based on how things are scoped.) If you're not careful and accidentally have logic involving self
in the wrong part of your decorator, you could have scoping issues.
The only advantages I can see to using functions inside of the classes are possibly because it is closer to the methods (but that introduces unnecessary nesting, potential scoping problems, and cognitive load of realizing that's a decorator and not a method), and better hiding of the function if it's name startswith _
or __
.
TL;DR Stylistic/Pythonicity concerns, and potential scoping issues.
Upvotes: 6