mardi 8 novembre 2016

How to refactor classes having potential for deep inheritance

I am stuck with the implementation of several classes which inherit abstract class AbstractStringConverter. I use Python 3.5.

AbstractStringConverter has an abstract method convert which converts given list of str into another list of str.

class AbstractStringConverter(metaclass=ABCmeta):
    @abstractmethod
    def convert(self, input, **kwargs):
        # convert input(list of str) to some list of str with some algorithm
        pass

Now I have 4 classes (StringConverterA, StringConverterB, StringConverterC, StringConverterD) inheriting AbstractStringConverter. All of these classes first find indices whose word should be converted (as you can see in find_indices_to_convert. Note that this function is defined outside the classes).

These classes can be categorized into 2 groups. First group (StringConverterA, StringConverterB, StringConverterC) converts a given input (again, this is a list of str) as follows:

  1. generate candidates which will replace the original word
  2. choose the best word among the candidates from 1.

In 2, each class will choose the best candidate by using several attributes (e.g., self.obj1, self.obj2, ...). So basically the difference among these classes occurs when choosing the best candidates.

Second group (StringConverterD) does not generate candidates but simply replace the original words with "FOO". In convert, it uses an external function find_indices_to_convert.

Toy code is shown below.

class StringConverterA(AbstractStringConverter):
    def __init__(self, obj1):
        self.obj1 = obj1

    def get_candidates(self, word):
        # find candidate words
        # `candidates` is a list of words
        return candidates

    def get_best_candidates(self, candidates):
        # logic for finding best candidate by using self.obj1
        return best_word

    def convert(self, input, **kwargs):
        result = list(input)
        indices = find_indices_to_convert(input)
        for index in indices:
             candidates = self.get_candidates(input[word])
             best_candidate = self.get_best_candidates(candidates)
             result[index] = best_candidate
        return best_candidate

class StringConverterB(AbstractStringConverter):
    def __init__(self, obj1, obj2):
        self.obj1 = obj1
        self.obj2 = obj2

    def get_candidates(self, word):
        # find candidate words
        # `candidates` is a list of words
        return candidates

    def get_best_candidates(self, candidates):
        # logic for finding best candidate by using self.obj1 and self.obj2
        return best_word

    def convert(self, input, **kwargs):
        result = list(input)
        indices = find_indices_to_convert(input)
        for index in indices:
             candidates = self.get_candidates(input[word])
             best_candidate = self.get_best_candidates(candidates)
             result[index] = best_candidate
        return best_candidate

class StringConverterC(AbstractStringConverter):
    def __init__(self, obj1, obj2, obj3, ...): # ... means many other parameters
        self.obj1 = obj1
        self.obj2 = obj2
        self.obj3 = obj3

    def get_candidates(self, word):
        # find candidate words
        # `candidates` is a list of words
        return candidates

    def get_best_candidates(self, candidates):
        # logic for finding best candidate by using self.obj1, self.obj2 and self.obj3
        return best_word

    def convert(self, input, **kwargs):
        result = list(input)
        indices = find_indices_to_convert(input)
        for index in indices:
             candidates = self.get_candidates(input[word])
             best_candidate = self.get_best_candidates(candidates)
             result[index] = best_candidate
        return best_candidate

class StringConverterD(AbstractStringConverter):
    def __init__(self, ...): # ... means many other parameters
        pass

    def convert(self, input, **kwargs):
        result = list(input)
        indices = find_indices_to_convert(input)
        for index in indices:
             result[index] = "FOO"

I didn't add find_indices_to_convert to AbstractStringConverter as its method, because I've heard it's not a good practice to use inheritance as a tool for sharing codes. Furthermore, I have a plan to implement other functions to find indices to convert. So I thought it would be better to make the function independent from the abstract class (just, intuitively).

My question is as follows: Does it make sense to make another abstract class like AbstractStringConverterWithCandidates which inherits AbstractStringConverter and has get_best_candidate as an abstract method, and make AbstractStringConverterD inherits directly from AbstractStringConverter? In this case, I fear that I am using inheritance in a wrong way, because I feel as if I am using inheritance just to share codes. And I am afraid that this will leads to deep inheritance if I want to implement other classes in the future.

Aucun commentaire:

Enregistrer un commentaire