Izen
Izen

Reputation: 21

Python - Refactor similar methods found in different classes

I'm in scenario where I want to refactor several classes which have identical and/or similar methods. The number of class are around ~20 and the number of similar methods are around ~15. All sorts of combinations exist within this space, which is why I'm a bit reluctant to using inheritance to solve this issue (rightfully?).

The code is part of a wrapper around another application that is controlled by a com api. The wrapper in turn is part of a package that is distributed internally at the company where I work. Therefore the interfaces of the classes have to remain the same (for backwards compatibility).

This example illustrates some very simplified versions of the classes:

class FirstCollectionLike:
    def __init__(self):
        self._collection = list()

    def add(self, arg):
        self._collection.append(arg)

    def remove(self, index):
        del self._collection[index]


class SecondCollectionLike:
    def __init__(self):
        self._collection = list()
        self._resource = some_module.get_resource()

    def start(self):
        some_module.start(self.resource)

    def add(self, arg):
        self._collection.append(arg)

    def remove(self, value):
        self._collection.remove(value)


class SomeOtherClass:
    def __init__(self):
        self._some_attribute = 0
        self._resource = some_module.get_resource()

    def add(self, value):
        self._some_attribute += value

    def start(self):
        some_module.start(self._resource)

Are there any design patterns I could look into that would help me solve this issue?

My initial thought was to create method classes like Add, RemoveByIndex and RemoveByName that implements __call__ like so:

class Add:
    def __init__(self, owner):
        self.owner = owner

    def __call__(self, item):
        self._collection.append(item)

class AddAndInstantiate:
    def __init__(self, owner, type_to_instantiate):
        self.owner = owner
        self.type_to_instantiate = type_to_instantiate

    def __call__(self, name):
        self._collection.append(type_to_instantiate(name))

and then assign instances of those classes as instance attributes to their respective owner objects:

class RefactoredClassOne:
    def __init__(self):
        self.add = Add(self)
        self.remove = RemoveByIndex(self)

class RefactoredClassTwo:
    def __init__(self):
        self.add = AddAndInstantiate(self, SomeClass)
        self.remove = RemoveByName(self)

This way I could quite easily add any method I want to a class and provide some arguments to the method class if needed (like the type of the class to instantiate in the example above). The downside is that it is a bit harder to follow what is happening, and the automatic documentation generation we use (sphinx) does not work if the methods are implemented in this way.

Does this seem like a bad approach? What are the alternatives?

Upvotes: 0

Views: 526

Answers (1)

Er...
Er...

Reputation: 653

First, if your classes are as simple as you example suggest, I'm not sure OOP is the right tool. What your classes are doing is just renaming a couple of basic calls. This is useless abstraction and IMO a bad practice (why force me to look to into the SecondClassCollectionLike.py file to discover that .add() is 1) in fact a wrongly named append and 2) that my collection is in fact a listwith a fancy name?)

In that case I'd say that a functional approach might be better, and a workflow such as:

a = SecondClassCollectionLike()
a.add("x")
a.add("y")
a.remove(0)
a.start()

would be a lot clearer if it looked like

a = list()
a.append("x")
a.append(y)
del a[0]
somemodule.start()

If your classes are in fact more complex and you really want to keep the OOP approach, I think that this solution is probably close to your solution and what you're looking for.

The idea is to have modules which hold the logic. For example a _collection_behaviour.py module, which holds the add(), remove(), increment() or whatever. And a _runtime.py module, which holds that start(), stop(), etc. logic.

This way you could have classes which exibit behaviour from these modules:

calss MyClass():
    def __init__(self):
        self._collection = list()

    from ._collection_behaviour import add
    from ._collection_behaviour import remove
    from ._runtime import start

But I do not see the point in making these functions classes which implement __call__ if that's all they do.

Upvotes: 1

Related Questions