10

I am trying to implement the Strategy design pattern to create an interface for an underlying algorithm to be implemented in a modular fashion.

Currently, as per code below, I have one top-level/parent abstract class (ParentAbstractStrategy) that defines the base interface for the strategy method.

I also have a one-level-down from this abstract class (ChildAbstractStrategy).

The reason I have two abstract classes is because of the attributes they need to hold; see the __init__ methods. ChildAbstractStrategy is a special case of ParentAbstractStrategy in that it stores an additional attribute: attr2. Otherwise its interface is identical, as seen by identical strategy method signatures.

Sometimes, I want to be able to directly subclass ParentAbstractStrategy and implement the strategy method (see ConcreteStrategyA), but other times I want to be able to subclass ChildAbstractStrategy, because the extra attribute is required (see ConcreteStrategyB).

An additional complication is that in some subclasses of either abstract class I want to be able to handle additional arguments in the strategy method. This is why I have added **kwargs to all signatures of the strategy method, so that I can pass in whatever additional arguments I want to a subclass, on a case-by-case basis.

This creates the last problem: these extra arguments are not optional in the subclasses. E.g. in the strategy method of ConcreteStrategyB I want to be certain that the caller passed in a third argument. I'm basically abusing **kwargs to provide what probably should be positional arguments (since I can't give them sane defaults and need their existence to be enforced).

This current solution of using **kwargs for "method overloading" in subclasses feels really messy, and I'm not sure if this means there is a problem with the class inheritance scheme or interface design, or both.

Is there a way that I can achieve these design goals in a cleaner fashion. It feels like I'm missing something big picture here and maybe the class/interface design is bad. Maybe creating two disjoint abstract classes with different signatures for the strategy method?

import abc


class ParentAbstractStrategy(metaclass=abc.ABCMeta):
    @abc.abstractmethod
    def __init__(self, attr1):
        self.attr1 = attr1

    @abc.abstractmethod
    def strategy(self, arg1, arg2, **kwargs):
        raise NotImplementedError


class ChildAbstractStrategy(ParentAbstractStrategy, metaclass=abc.ABCMeta):
    @abc.abstractmethod
    def __init__(self, attr1, attr2):
        super().__init__(attr1)
        self.attr2 = attr2

    @abc.abstractmethod
    def strategy(self, arg1, arg2, **kwargs):
        raise NotImplementedError


class ConcreteStrategyA(ParentAbstractStrategy):
    def __init__(self, attr1):
        super().__init__(attr1)

    def strategy(self, arg1, arg2, **kwargs):
        print(arg1, arg2)


class ConcreteStrategyB(ChildAbstractStrategy):
    def __init__(self, attr1, attr2):
        super().__init__(attr1, attr2)

    def strategy(self, arg1, arg2, **kwargs):
        print(arg1, arg2)
        arg3 = kwargs.get("arg3", None)

        if arg3 is None:
            raise ValueError("Missing arg3")
        else:
            print(arg3)

Here's an interpreter session demonstrating how it's currently working:

>>> a = ConcreteStrategyA(1)
>>> a.attr1
1
>>> a.strategy("a", "b")
a b
>>> b = ConcreteStrategyB(1, 2)
>>> b.attr1
1
>>> b.attr2
2
>>> b.strategy("a", "b")
a b
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/space/strategy.py", line 42, in strategy
    raise ValueError("Missing arg3")
ValueError: Missing arg3
>>> b.strategy("a", "b", arg3="c")
a b
c
eyllanesc
  • 235,170
  • 19
  • 170
  • 241
Space
  • 183
  • 1
  • 7

1 Answers1

6

Answering my own question.

My usage of **kwargs is 'bad' in this scenario. Why? As far as I can tell, **kwargs is typically used for:

  1. Wrapper functions, e.g. decorators.
  2. Collecting extra keyword arguments to a function that the function knows about (e.g. see usage in https://matplotlib.org/api/_as_gen/matplotlib.pyplot.plot.html?highlight=plot#matplotlib.pyplot.plot). In this scenario the **kwargs are optional arguments that can be passed into the function and they have sane default values.

Making **kwargs to be required in a function call is defeating their purpose; positional arguments that need to be explicitly supplied should be used instead. That way the interface provided by the function has to be explicitly satisfied by the caller.

There is another problem with using **kwargs in an interface, as I have. It involves the LSP (Liskov Substitution Principle, see https://en.wikipedia.org/wiki/Liskov_substitution_principle). The current implementation is abusing **kwargs in an attempt to define a variable interface for the strategy method among sublcasses. Although syntactically the function signatures for all strategy methods match, semantically the interfaces are different. This is a violation of the LSP, which would require that I could e.g. treat any descendant of ParentAbstractStrategy the same when considering their interfaces, e.g. I should be able to treat the strategy method of ConcreteStrategyA and ConcreteStrategyB the same.

What was my solution? I have changed the interface of the strategy method to no longer include **kwargs and instead use a mix of positional arguments and keyword arguments with default values. E.g. if ConcreteStrategyB still needs a third argument arg3 but ConcreteStrategyA does not, I could change the classes to look like this:

class ConcreteStrategyA(ParentAbstractStrategy):
    def __init__(self, attr1):
        super().__init__(attr1)

    def strategy(self, arg1, arg2, arg3=None):
        print(arg1, arg2)


class ConcreteStrategyB(ChildAbstractStrategy):
    def __init__(self, attr1, attr2):
        super().__init__(attr1, attr2)

    def strategy(self, arg1, arg2, arg3=None):
        print(arg1, arg2)
        assert arg3 is not None
        print(arg3)

With the interfaces of both parent classes changed to match.

Space
  • 183
  • 1
  • 7
  • Thanks for sharing your findings. I'm in a similar pickle trying to implement different data filtering strategies, for most of which entry date doesn't matter while for few others it does. I guess I'll need to drag `date=None` for my concrete strategies – lotrus28 Jul 25 '23 at 07:38