tags:

views:

131

answers:

1

I have two classes that I would like to merge into a composite. These two classes will continue to be used standalone and I don't want to modify them. For some reasons, I want to let my composite class creating the objects. I am thinking about something like the code below (it is just an example) but I think it is complex and I don't like it very much. I guess that it could be improved by some techniques and tricks that I ignore.

Please note that the composite is designed to manage a lot of different classes with different constructor signatures.

What would recommend in order to improve this code?

class Parent:
    def __init__(self, x):
        self.x = x

class A(Parent):
    def __init__(self, x, a="a", b="b", c="c"):
        Parent.__init__(self, x)
        self.a, self.b, self.c = a, b, c

    def do(self):
        print self.x, self.a, self.b, self.c

class D(Parent):
    def __init__(self, x, d):
        Parent.__init__(self, x)
        self.d = d

    def do(self):
        print self.x, self.d

class Composite(Parent):
    def __init__(self, x, list_of_classes, list_of_args):
        Parent.__init__(self, x)
        self._objs = []
        for i in xrange(len(list_of_classes)):
            self._objs.append(self._make_object(list_of_classes[i], list_of_args[i]))

    def _make_object(self, the_class, the_args):
        if the_class is A:
            a = the_args[0] if len(the_args)>0 else "a"
            b = the_args[1] if len(the_args)>1 else "b"
            c = the_args[2] if len(the_args)>2 else "c"
            return the_class(self.x, a, b, c)
        if the_class is D:
            return the_class(self.x, the_args[0])

    def do(self):
        for o in self._objs: o.do()


compo = Composite("x", [A, D, A], [(), ("hello",), ("A", "B", "C")])
compo.do()
+3  A: 

You could shorten it by removing type-checking _make_object, and letting class constructors take care of the default arguments, e.g.

class Composite(Parent):
    def __init__(self, x, list_of_classes, list_of_args):
        Parent.__init__(self, x)
        self._objs = [
            the_class(self.x, *the_args)
            for the_class, the_args
            in zip(list_of_classes, list_of_args)
            if isinstance(the_class, Parent.__class__)
        ]

    def do(self):
        for o in self._objs: o.do()

This would also allow you to use it with new classes without modifying its code.

PiotrLegnica
'if isinstance(the_class, Parent.__class__)' should be replaced by 'if issubclass(the_class, Parent)'. except this minor fix, it works great and corresponds to the kind of improvement I am looking for. Thanks
luc