Abstract Parent and Inheritance vs. Strategy Pattern and Composition

Question

I'm currently refactoring a class that looks (after some refactoring and very much simplified) somewhat like this:

class Foo
{
  public:
    Foo(bool someFlag) : m_flag(someFlag) { };

    void doThings();
    void doOtherThings()
    {
        doPrivateThings();
        if (m_flag)
            doFlagThings();
        else
            doNoFlagThings();
    }

  private:
    void doPrivateThings();
    void doFlagThings();
    void doNoFlagThings();
    bool m_flag;
}

Notes:
* m_flag being there is the result of the refactoring (which is by far not done) - initially each function individually branched (if (check_something())) on something that for which the result was already determined/fixed during construction of the object.
* The functions doFlagThings() and doNoFlagThings() are rather small (~3-5 lines of code) and there are at most 3-4 functions that deviate depending of m_flag.

So, the next step is to get rid of the someFlag - the only question is how.
One way would be to use an abstract parent and inheritance:

class FooBase
{
  public:
    FooBase();

    void doThings();
    void doOtherThings()
    {
        doPrivateThings();
        doSpecializedThings();
    }

  private:
    void doPrivateThings();
    virtual void doSpecializedThings() = 0;
}

class FlagFoo : public FooBase
{
  public:
    FlagFoo();

  private:
    void doSpecializedThings() override { ... };
}

class NoFlagFoo : public FooBase
{
  public:
    NoFlagFoo();

  private:
    void doSpecializedThings() override { ... };
}

The other way would be to use the Strategy pattern and composition, i.e.

class Foo
{
  public:
    Foo(IDoThingsStrategystrategy strategy) : m_Strategy(strategy) { };

    void doThings();
    void doOtherThings()
    {
        doPrivateThings();
        m_Strategy.doSpecializedThings();
    }

  private:
    void doPrivateThings();
    IDoThingsStrategy m_Strategy;
}

class IDoThingsStrategy
{
  public:
    IDoThingsStrategy();
    virtual void doSpecializedThings() = 0;
}

class DoThingsFlagStrategy : public IDoThingsStrategy
{
  public:
    DoThingsFlagStrategy();
    void doSpecializedThings() override;
}

class DoThingsNoFlagStrategy : public IDoThingsStrategy
{
  public:
    DoThingsNoFlagStrategy();
    void doSpecializedThings() override;
}

But that seems very much like overkill, and presents the problem that the creator of Foo now needs to be aware of the different Strategies in order to create them and feed them to Foo.

So the question is: Should I go with the Inheritance (even though one should prefer composition over inheritance) or the Strategy pattern or is there a better way I'm just not seeing right now?

Answer 1

In short

Your code is the typical example for applying the template method design pattern:

Define the skeleton of an algorithm in an operation, deferring some steps to a subclass. Template methtods lets subclasses redefine certain steps of an algorithm without changing the algorithm structure.
- E.Gamma, R.Helm, R.Johnson, J.Vlissides (aka the GoF)

So go for your first refactoring option, because it's exactly what you intend to do.

More explanations**

Your current code of doFlagThings() and doNoFlagThings() may perfectly well use protected or private members, because currently they are both member functions.

With the template method pattern, you wouldn't bother about this (except perhaps some private members that would have to become protected when refactored in the FooBase class.

THe strategy template intends to encapsulate a family of alogrithm to make them usable interchangeably. In your case:

• if the code in doFlagThings() and doNoFlagThings() would be very independent of the rest of the class, it would certainly be a valid alternative;
• However, if it's not, you'd need to make the strategy classes friend of Foo, and in addition pass the calling object as parameter so that the strategy can invoke the right functions and access to the right member variables. So you'd have a false strategy, where you'd have a very strong coupling between all the classes. Not exactly the intent of the pattern...

But why do you hesitate ? You've identified by yourself that the strategy would be overkill in your situation. Is it perhap's because you have the mantra "composition over inheritance" in your head ? Then be reassured: it's just a rule of thumb and not a golden rule. There are cases where inheritance really is the best solution (otherwhise there would be no inheritance in OOP ;-) )

Answer 2

Your composition strategy still involves inheritance with virtual methods, so that really doesn't simplify over the (first) direct inheritance option.  Thus, given the choice between the two, the inheritance seems simpler.  And you can always refactor again later if you need to compose.

---

The main factor in my mind is to first and foremost make things simple for the consuming clients.  In this case, a good choice would seem to be using a factory method so that callers don't have to know whether they are supposed to new some specific subclasses, or, use a strategy pattern, injected into the constructor.  This way you can refactor the implementation later without changing those clients consuming the abstraction.

---

Sometimes, as food for thought, if we take a view of larger context, we can find yet other alternatives.  In particular, if one is dealing with a set of such objects, we can use two different collections: one for flag objects that should do the flag thing, and one for no-flag objects that shouldn't.  Here by grouping the flag and no-flag objects together we are sometimes able to streamline processing and differentiation between these objects, by processing all of the flag objects together, and processing all of the no-flag objects together.  So, now the choice is which collection to add the object to, rather than which subclass to create.  Of course, given the simplified expression of the problem, one cannot tell if this would apply, so this is just food for thought.

Answer 3

An important key to good design is the ability to see into the future. You need to predict (and you will be judged on how well you predict) how the code will evolve.

You want MORE generalization where you expect the code to split up more, and become more complex. You generally need less generalization where the code wont change much.

A key flaw with your 'anonymizing' names so they don't mean anything, is that I cannot given GUESS where you will be wanted to generalize more in the future. You have to figure that out.

The code you have now - with conditionals (m_flag) - really isn't bad, depending on how you expect the code to evolve over time. But if you expect lots of doOtherThings() implementations depending on subtype - you see that as a productive area of change - then subclassing is a good idea.

What you described as IDoThingsStrategy does NOT seem like a particular good idea - at least as far as GENERIZING codes. It is a good strategy for MODULARIZING (but the reader - me - doesnt know if it makes sense for modularizing). What you are doing is allowing the subclassing to happen separated and without respect to the other data in 'Foo'. That could be a PITA (if you generally need that information) or smart, if you dont.