is decorator the right thing to use?

[Bruno Desthuilliers]

Dmitry S. Makovey a écrit :
> Thanks Bruno,
> 
> your comments were really helpful (so was the "improved" version of code). 
> 
> My replies below:
> 
> Bruno Desthuilliers wrote:
>>> So decorators inside of B just identify that those methods will be
>>> proxied by A. On one hand from logical standpoint it's kind of weird to
>>> tell class that it is going to be proxied by another class,
 >>>
>> Indeed - usually, proxied objects shouldn't have to be aware of the
>> fact. That doesn't mean your variation on the proxy pattern is
>> necessarily bad design (hard to tell without lot of context anyway...),
>> but still there's some alarm bell ringing here IMHO - IOW : possibly the
>> right thing to do, but needs to be double-checked.
> 
> I'm kind of looking at options and not dead-set on decorators, but I can't
> find any other "elegant enough" solution which wouldn't lead to such tight
> coupling. The problem I'm trying to solve is not much more complicated than
> what I have already described 

Well... You didn't mention why you need a proxy to start with !-)


> so if anybody can suggest a better approach -
> I'm all for it. 
> 
(snip code)

>> My point is that:
>> 1/ you shouldn't have to rewrite a decorator function - with basically
>> the same code - for each possible proxy class / attribute name pair combo
>> 2/ making the decorator an attribute of the proxy class makes
>> dependencies clearer (well, IMHO at least).
> 
> agreed on all points
> 
>> I'm still a bit uneasy wrt/ high coupling between A and B, and if I was
>> to end up with such a design, I'd probably take some times to be sure
>> it's really ok.
> 
> that is the question that troubles me at this point - thus my original post
> (read the subject line ;) ).  I like the clarity decorators bring to the
> code and the fact that it's a solution pretty much "out-of-the-box" without
> need to create something really-really custom, but I'm worried about tight
> coupling and somewhat backward logic that they would introduce (the way I
> envisioned them).

Well... The canonical solution for delegation in Python is using 
__getattr__. Your problem - according to this post and your answer to 
Diez - is that your proxy may have to
1/ delegate to more than one object
2/ don't necessarily delegate each and any attribute access

I can envision one solution using both __getattr__ and a simple decorator:

def proxy(func):
    func._proxied = True
    return func

class A(object):
     def __init__(self, delegates):
         self._delegates = delegates
     def __getattr__(self, name):
         for d in self.__delegate:
             func = getattr(d, name)
             if callable(func) and getattr(func, '_proxied', False):
                 return func
         raise AttributeError(
               'object %s has no attribute '%s' % (self.__class__, name)
               )


class B(object):
     def __init__(self):
         self.val='bval'

     @proxy
     def bmethod(self,a):
         print "B::bmethod"
         print a, self.val

     @proxy
     def bmethod2(self,a):
         print "B::bmethod2"
         print a, self.val


class C(object):
     def __init__(self):
         self.val='bval'

     @proxy
     def cmethod(self,a):
         print "B::bmethod"
         print a, self.val

     @proxy
     def cmethod2(self,a):
         print "B::bmethod2"
         print a, self.val


a = A([B(), C()])

# not tested...


This solves most of the coupling problems (B and C still have to make 
clear which methods are to be proxied, but at least they need not know 
which class will be used as proxy), and makes sure only 'allowed' method 
calls are delegated. But I wouldn't call it a perfect solution neither. 
If you do have more than one object having method xxx, only the first 
one will match... And let's not talk about the lookup penalty.

There's a possible variant that avoids the call to __getattr__ (in 
short: attaching delegation instancemethods to A instance in the 
initializer for each proxied method in delegates), but that wont solve 
the problem of potential name clashes.


My 2 cents...