Using composition instead of inheritance and abstract classes

Question

I have a tool that displays data in some format. For example, it can display a list of dogs in a CSV or JSON formats. In the code, I had a lot of places where I had to check the format and display the data in that format. The only difference between the code blocks is the calls to the data. Some of them call get_name() and get_id() while others call get_type() and so on.

I tried to create an abstract class that has the display method and the derived classes have the get_block (for JSON), get_line (for CSV) and so on. The code looks like:

class DisplayingFormatEnum(Enum):
 OPTION_A = 'user-option-a'
 OPTION_B = 'user-option-b'
class DisplayingFormat(ABC):
 def get_row(data):
 pass
 def get_block(data):
 pass
 def get_line(data):
 pass
 def display_format_table(self):
 table = PrettyTable(self.title)
 for d in self.data:
 table.add_row(self.get_row(d))
 print(table)
 def print_json(self):
 result = []
 for d in self.data:
 block = self.get_block(d)
 result.append(block)
 print(result)
 def display_format_dsv(self):
 for d in self.data:
 print(f"{self.get_line(d)}")
 def display_format_list(self):
 if self.result_format == 'table':
 self.display_format_table()
 elif self.result_format == 'json':
 self.display_format_json()
 else:
 self.display_format_dsv()
class DisplayingFormatPeople(DisplayingFormat):
 def __init__(self, people, result_format, enum_type):
 if not isinstance(enum_type, DisplayingFormatEnum):
 raise DisplayingFormatError("Entity must be an instance of DisplayingFormatEnum Enum")
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['people']
 else:
 raise DisplayingFormatError(f"DisplayingFormatPeople can only use {DisplayingFormatEnum.OPTION_A.value} as a valid enum_type. Invalid enum_type: {enum_type.value}")
 self.result_format = result_format
 self.data = people
 self.enum_type = enum_type
 def get_row(self, person):
 return [person.get_name(), person.get_id()]
 def get_block(self, person):
 block = {}
 block[self.title[0]] = person.get_name()
 block[self.title[1]] = person.get_id()
 return block
 def get_line(self, person):
 return ';'.join((person.get_name(), person.id()))
class DisplayingFormatCars(DisplayingFormat):
 def __init__(self, cars, result_format, enum_type):
 if not isinstance(enum_type, DisplayingFormatEnum):
 raise DisplayingFormatError("Entity must be an instance of DisplayingFormatEnum Enum")
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['cars']
 else:
 raise DisplayingFormatError(f"DisplayingFormatProjects can only use {DisplayingFormatEnum.OPTION_A.value} as a valid enum_type. Invalid enum_type: {enum_type.value}")
 self.result_format = result_format
 self.data = cars
 self.enum_type = enum_type
 def get_row(self, car):
 return [car.get_type()]
 def get_block(self, car):
 block = {}
 block[self.title[0]] = car.get_type()
 return block
 def get_line(self, car):
 return car.get_type()
class DisplayingFormatDogs(DisplayingFormat):
 def __init__(self, dogs, result_format, enum_type):
 if not isinstance(enum_type, DisplayingFormatEnum):
 raise DisplayingFormatError("Entity must be an instance of DisplayingFormatEnum Enum")
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['dogs']
 elif enum_type.value == DisplayingFormatEnum.OPTION_B.value:
 self.title = TITLE_OPTION_B['dogs']
 else:
 error_line = "DisplayingFormatDogs can only use {DisplayingFormatEnum.OPTION_A.value} and {DisplayingFormatEnum.OPTION_B.value} as valid entities."
 error_line += "Invalid enum_type: {enum_type.value}"
 raise DisplayingFormatError(f"{error_line}")
 self.result_format = result_format
 self.data = dogs
 self.enum_type = enum_type
 def get_row(self, dog):
 return [dog.get_nickname(), dog.get_type()]
 def get_block(self, dog):
 block = {}
 block[self.title[0]] = dog.get_nickname()
 block[self.title[2]] = dog.get_type()
 return block
 def get_line(self, dog):
 return ';'.join((dog.get_nickname(), dog.get_type()))
class DisplayingFormatError(Exception):
 pass

I believe that this code is a good use of inheritance but I think it does not fit well with the "composition over inheritance" rule. I'm afraid of unexpexted future changes that will require me to change this code. How should I change my code to follow this rule?

• Toby Speight: Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks!

Answer 1

I'm not convinced using an abstract base class is really useful here. All of your methods follow exactly the same principles. The only differences are the names and fields and the allowed enumeration values.

So as a first step I would put all the code into the baseclass, and only add the check for the enumeration into the constructor and implement the get_data method in each subclass:

class DisplayingFormat:
 def __init__(self, data, result_format, enum_type):
 if not isinstance(enum_type, DisplayingFormatEnum):
 raise DisplayingFormatError("Entity must be an instance of DisplayingFormatEnum Enum")
 self.result_format = result_format
 self.data = data
 self.enum_type = enum_type
 self.title = None
 def get_data(self, obj):
 raise NotImplementedError
 def to_table(self):
 table = PrettyTable(self.title)
 for d in self.data:
 table.add_row(self.get_data(d))
 return table
 def to_json(self):
 return [dict(zip(self.title, self.get_data(d))) for d in self.data]
 def to_dsv(self):
 return [';'.join(map(str, self.get_data(d))) for d in self.data]
 def display_format_list(self):
 if self.result_format == 'table':
 print(self.to_table())
 elif self.result_format == 'json':
 print(self.to_json())
 else:
 print(self.to_dsv())
class DisplayingFormatPeople(DisplayingFormat):
 def __init__(self, people, result_format, enum_type):
 super().__init__(people, result_format, enum_type)
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['people']
 else:
 raise DisplayingFormatError(f"DisplayingFormatPeople can only use {DisplayingFormatEnum.OPTION_A.value} as a valid enum_type. Invalid enum_type: {enum_type.value}")
 def get_data(self, person):
 return person.get_name(), person.get_id()
class DisplayingFormatCars(DisplayingFormat):
 def __init__(self, cars, result_format, enum_type):
 super().__init__(cars, result_format, enum_type)
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['cars']
 else:
 raise DisplayingFormatError(f"DisplayingFormatProjects can only use {DisplayingFormatEnum.OPTION_A.value} as a valid enum_type. Invalid enum_type: {enum_type.value}")
 def get_data(self, car):
 return car.get_type()
class DisplayingFormatDogs(DisplayingFormat):
 def __init__(self, dogs, result_format, enum_type):
 super().__init__(cars, result_format, enum_type)
 if enum_type.value == DisplayingFormatEnum.OPTION_A.value:
 self.title = TITLE_OPTION_A['dogs']
 elif enum_type.value == DisplayingFormatEnum.OPTION_B.value:
 self.title = TITLE_OPTION_B['dogs']
 else:
 error_line = f"DisplayingFormatDogs can only use {DisplayingFormatEnum.OPTION_A.value} and {DisplayingFormatEnum.OPTION_B.value} as valid entities."
 error_line += f"Invalid enum_type: {enum_type.value}"
 raise DisplayingFormatError(error_line)
 def get_data(self, dog):
 return dog.get_nickname(), dog.get_type()

Note that I also fixed some un-/needed f-strings and used list comprehensions where applicable. I also modified and renamed your format methods so they return the formatted object and are only printed in display_format_list, which might need a better name as well.

This could be simplified further. The easiest way is if you can modify the objects in data to have an implemented __getitem__ method, but you can also hack it together using getattr. What you really only need for each subclass are the field names and the association between the option and the title:

class DisplayingFormat:
 fields = []
 titles = {}
 def __init__(self, data, result_format, enum_type):
 if not isinstance(enum_type, DisplayingFormatEnum):
 raise DisplayingFormatError("Entity must be an instance of DisplayingFormatEnum Enum")
 if enum_type not in self.titles:
 raise DisplayingFormatError(f"enumy_type must be in {[opt.value for opt in self.titles]}. Invalid enum_type: {enum_type.value}")
 self.result_format = result_format
 self.data = data
 self.title = self.titles[enum_type]
 def get_data(self, obj):
 # if you implement __getitem__
 return [obj[f] for f in self.fields]
 # or this, if you cannot modify your objects interface
 return [getattr(obj, f"get_{f}")() for f in self.fields]
 ...
class DisplayingFormatPeople(DisplayingFormat):
 fields = ["name", "id"]
 titles = {DisplayingFormatEnum.OPTION_A: TITLE_OPTION_A['people']}
class DisplayingFormatCars(DisplayingFormat):
 fields = ["type"]
 titles = {DisplayingFormatEnum.OPTION_A: TITLE_OPTION_A['dogs']}
class DisplayingFormatDogs(DisplayingFormat):
 fields = ["nickname", "type"],
 titles = {DisplayingFormatEnum.OPTION_A: TITLE_OPTION_A['dogs']
 DisplayingFormatEnum.OPTION_B: TITLE_OPTION_B['dogs']}

If you really want to go the route of composition, you would probably want to make the formatter an argument, so that you have only one class with this interface:

class Display:
 def __init__(self, data, title, formatter):
 ...

where taking care of selecting the proper columns to go to data as well as selecting the proper title is all external to the class. The formatter then takes the role of the to_json, ... methods. And at that point it doesn't even need to be a class anymore, a function would suffice.

• vesii: I don't get how to_json and to_dsv work. What if get_data returns only one string and not tuple? it prints something like p;e;r;s;o;n.