Composition vs. inheritance

Contrarian view on composition over inheritance.

style
Author

A. Coady

Published

November 18, 2023

Contrarian view on composition over inheritance.

The conventional wisdom is to prefer composition over inheritance. More specifically to use delegation over single inheritance.

Like the recommendation on closing files, the advice is well-intentioned but omits the fact that Python does not support it well. Python has no mechanism for embedding or forwarding methods. And the despite its famous duck-typing, there are many cases where a type must be subclassed to be substitutable (particularly if implemented in CPython).

The below example comes from a popular PyCon talk called Beyond PEP 8. The goal is to adapt a Java-esque interface into pythonic code.

Original implementation

import jnettool.tools.elements.NetworkElement
import jnettool.tools.Routing
import jnettool.tools.RouteInsector

ne = jnettool.tools.elements.NetworkElement('171.0.2.45')

try:
    routing_table = ne.getRoutingTable()
except jnettool.tools.elements.MissingVar:
    logging.exception('No routing table found')
    ne.cleanup('rollback')
else:
    num_routes = routing_table.getSize()
    for RToffset in range(num_routes):
        route = routing_table.getRouteByIndex(RToffset)
        name = route.getName()
        ipaddr = route.getIPAddr()
        print "%15s -> %s" % (name, ipaddr)
finally:
    ne.cleanup('commit')
    ne.disconnect()

Proposed interface

from nettools import NetworkElement

with NetworkElement('171.0.2.45') as ne:
    for route in ne.routing_table:
        print "%15s -> %s" % (route.name, route.ipaddr)

Proposed solution

import jnetool.tools.elements.NetworkElement
import jnetool.tools.Routing

class NetworkElementError(Exception):
    pass

class NetworkElement(object):

    def __init__(self, ipaddr):
        self.ipaddr = ipaddr
        self.oldne = jnetool.tools.elements.NetworkElement(ipaddr)

    @property
    def routing_table(self):
        try:
            return RoutingTable(self.oldne.getRoutingTable())
        except jnetool.tools.elements.MissingVar:
            raise NetworkElementError('No routing table found')

    def __enter__(self):
        return self

    def __exit__(self, exctype, excinst, exctb):
        if exctype == NetworkElementError:
            logging.exception('No routing table found')
            self.oldne.cleanup('rollback')
        else:
            self.oldne.cleanup('commit')
        self.oldne.disconnect()

    def __repr__(self):
        return '%s(%r)' % (self.__class__.__name__, self.ipaddr)


class RoutingTable(object):

    def __init__(self, oldrt):
        self.oldrt = oldrt

    def __len__(self):
        return self.oldrt.getSize()

    def __getitem__(self, index):
        if index >= len(self):
            raise IndexError
        return Route(self.oldrt.getRouteByIndex(index))


class Route(object):

    def __init__(self, old_route):
        self.old_route = old_route

    @property
    def name(self):
        return self.old_route.getName()

    @property
    def ipaddr(self):
        return self.old_route.getIPAddr()

No dispute that the interface is superior, but the implementation is using delegation as if it is dogma. The usage pattern has to be extrapolated from one example, but here are the issues:

  • Custom exceptions are not helpful if they do nothing. The consumer of this code does not use NetworkElementError, and has lost the traceback if it did. Error hiding is not error handling.
  • Comparing classes with == is widely considered an anti-pattern, as opposed to is or issubclass.
  • The Route object doesn’t need to delegate. There is no reason to assume that the underlying attribute access must be lazy, particularly since the iteration could be lazy instead. A named tuple or dataclass would suffice here.
  • The RoutingTable object doesn’t need to delegate. There is no need to support random access or lazy evaluation. Its only addition to the interface is to be sequence-like, which could be trivially accomplished by a sequence.
  • The NetworkElement doesn’t need to delegate. It has the same name, same constructor, a repr designed to appear as the original, and only extends behavior. If this doesn’t pass as an is-a relation, nothing does.

Simple solution

import collections
from jnettool.tools import elements

Route = collections.namedtuple('Route', ['name', 'ipaddr'])

class NetworkElement(elements.NetworkElement):
    @property
    def routing_table(self):
        table = self.getRoutingTable()
        routes = map(table.getRouteByIndex, range(table.getSize()))
        return [Route(route.getName(), route.getIPAddr()) for route in routes]

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        if isinstance(exc_val, elements.MissingVar):
            logging.exception("No routing table found")
            self.cleanup('rollback')
        else:
            self.cleanup('commit')
        self.disconnect()

Which version is more maintainable? Surely the simpler one.

Which version is more extensible? Well, by whom? The implementor can extend either just as easily. The caller can use the inherited version without losing any functionality.

So a better question might be which version is more flexible or reusable? Surely the inherited version, because the delegated version would need to access oldne. Even naming the delegate is a pain point, because one has to decide if it is a part of the public interface or not. Should it have 0, 1, or 2 leading underscores? Delegation is often touted as achieving both encapsulation and extensibility, despite being opposing goals.

Finally, there is also a simpler interface, again with the caveat that there is only one usage example. An iterable of 2-field objects, one of which is called name, and “points to” the other field. Sounds like a mapping.

class NetworkElement(elements.NetworkElement):
    @property
    def routing_table(self):
        table = self.getRoutingTable()
        routes = map(table.getRouteByIndex, range(table.getSize()))
        return {route.getName(): route.getIPAddr() for route in routes}
    ...