E pur si muove

Decorators specific to a class

Wednesday, December 30, 2009

My gut tells me it's horribly wrong but I am failing to formulate a decent argument, let me show an example what I mean (somewhat contrived):

def deco(f):
    def wrapper(self, *args, **kw):
        with self.lock:
            f(self, *args, **kw)

class Foo:
    def __init__(self):
        self.lock = threading.Lock()

    def method(self):

Here the decorator knows something about the arguments of the function it will end up calling, the first argument is "self" and it is an object with a "lock" attribute which is a context manager. Somehow I feel like that's more knowledge about the wrapped object then a decorator should have. It just is an indirection of logic my bain doesn't cope with.

There are obviously places where I could construct something like this. But I do never naturally think of doing it that way, I always end up with some other way which I find more elegant and I think the resulting logic is easier to follow. It's just that whenever I encounter code like this my brain starts hurting and I'm not sure I have a decent argument to tell people writing code like this off (you can hardly regard "it's a level of logic indirection that makes my brain hurt" as an argument).

Wednesday, December 30, 2009 | Labels: |


Paul said...

I often use class-level decorators -- just define them in the class block to make it explicit that they're tied to a class.

Jack Diederich said...

Is the lock really per-instance? If it is there isn't an obvious better way to do it; because the decorator is at the class level but the lock doesn't exist until there is an instance.

You could put the logic into a descriptor instead, but that's at least as much indirection as the decorator.

Floris Bruynooghe said...

Defining the decorator as a method of the class itself would probably make me happy.

The lock (or rather "the object", I said the example was a bit contrived) is not always at class level, sometimes it's an object passed into the contructor or something. But even if it is an instance attribute I prefer some other way of doing this, e.g. you could start method() with "self.do_the_deco_stuff()" which is more what I tend to do if a decorator would need knowledge of the object it acts on.

I think it's because the decorator tries to simply do stuff with the object rather then wrap the function call with something fancy that makes it seem wrong to me.

Tobias said...

I've dealt with similar situations by going through the indirection of a property:

def lock_on(lock_prop):
  def deco(func):
    def wrapper(self, *args, **kw):
      with lock_prop.__get__(self):
        return func(self, *args, **kw)
    return wrapper
  return deco

Then use it like this:

class Foo:
  def __init__(self):
    self._lock = threading.Lock()
  lock = property(lambda self:self._lock)
  def method(self):

rgz said...

Honestly I see nothing wrong with that decorator, what is its sin? That it only applies to methods? that it knows a little about the arguments it's going to receive? don't `property` and `classmethod` do the same?

Clearly it's ok, or would methods suddenly stop receiving an object as first parameter?

Of course not, there's nothing wrong with that decorator.

Nor is it clear to me why should that decorator become part of the class, it is NOT specific to that class, it works for any class with a lock property.

If you put that decorator into a class then at least put it inside a superclass so that you can use it again in other lock bearing objects.

OG's Monkey said...

I see nothing especially wrong with this as presented. Function deco expects its argument to be a class instance with context manager attribute called lock. This looks like duck typing to me - 'if it looks an object with a context manager called lock then it is...'.

I imagine the context is all. In some situations this might well be a very elegant solution and in others, very questionable.

New comments are not allowed.

Subscribe to: Post Comments (Atom)