oop - Diamond inheritance (C++)

Question

I know that having diamond inheritance is considered bad practice. However, I have 2 cases in which I feel that diamond inheritance could fit very nicely. I want to ask, would you recommend me to use diamond inheritance in these cases, or is there another design that could be better.

Case 1: I want to create classes that represent different kinds of "Actions" in my system. The actions are classified by several parameters:

• The action can be "Read" or "Write".
• The action can be with delay or without delay (It is not just 1 parameter. It changes the behavior significantly).
• The action's "flow type" can be FlowA or FlowB.

I intend to have the following design:

// abstract classes
class Action  
{
    // methods relevant for all actions
};
class ActionRead      : public virtual Action  
{
    // methods related to reading
};
class ActionWrite     : public virtual Action  
{
    // methods related to writing
};
class ActionWithDelay : public virtual Action  
{
    // methods related to delay definition and handling
};
class ActionNoDelay   : public virtual Action  {/*...*/};
class ActionFlowA     : public virtual Action  {/*...*/};
class ActionFlowB     : public virtual Action  {/*...*/};

// concrete classes
class ActionFlowAReadWithDelay  : public ActionFlowA, public ActionRead, public ActionWithDelay  
{
    // implementation of the full flow of a read command with delay that does Flow A.
};
class ActionFlowBReadWithDelay  : public ActionFlowB, public ActionRead, public ActionWithDelay  {/*...*/};
//...

Of course, I will obey that no 2 actions (inheriting from Action class) will implement the same method.

Case 2: I implement the composite design pattern for a "Command" in my system. A command can be read, written, deleted, etc. I also want to have a sequence of commands, which can also be read, written, deleted, etc. A sequence of commands can contain other sequences of commands.

So I have the following design:

class CommandAbstraction
{
    CommandAbstraction(){};
    ~CommandAbstraction()=0;
    void Read()=0;
    void Write()=0;
    void Restore()=0;
    bool IsWritten() {/*implemented*/};
    // and other implemented functions
};

class OneCommand : public virtual CommandAbstraction
{
    // implement Read, Write, Restore
};

class CompositeCommand : public virtual CommandAbstraction
{
    // implement Read, Write, Restore
};

In addition, I have a special kind of commands, "Modern" commands. Both one command and composite command can be modern. Being "Modern" adds a certain list of properties to one command and composite command (mostly same properties for both of them). I want to be able to hold a pointer to CommandAbstraction, and initialize it (via new) according to the needed type of command. So I want to do the following design (in addition to the above) :

class ModernCommand : public virtual CommandAbstraction
{
    ~ModernCommand()=0;
    void SetModernPropertyA(){/*...*/}
    void ExecModernSomething(){/*...*/}
    void ModernSomethingElse()=0;

};
class OneModernCommand : public OneCommand, public ModernCommand
{
    void ModernSomethingElse() {/*...*/};
    // ... few methods specific for OneModernCommand
};
class CompositeModernCommand : public CompositeCommand, public ModernCommand
{
    void ModernSomethingElse() {/*...*/};
    // ... few methods specific for CompositeModernCommand
};

Again, I will make sure that no 2 classes inheriting from CommandAbstraction class will implement the same method.

Thank you.

Answer 1

Inheritance is the second strongest (more coupling) relations in C++, preceded only by friendship. If you can redesign into using only composition your code will be more loosely coupled. If you cannot, then you should consider whether all your classes should really inherit from the base. Is it due to implementation or just an interface? Will you want to use any element of the hierarchy as a base element? Or are just leaves in your hierarchy that are real Action's? If only leaves are actions and you are adding behavior you can consider Policy based design for this type of composition of behaviors.

The idea is that different (orthogonal) behaviors can be defined in small class sets and then bundled together to provide the real complete behavior. In the example I will consider just one policy that defines whether the action is to be executed now or in the future, and the command to execute.

I provide an abstract class so that different instantiations of the template can be stored (through pointers) in a container or passed to functions as arguments and get called polymorphically.

class ActionDelayPolicy_NoWait;

class ActionBase // Only needed if you want to use polymorphically different actions
{
public:
    virtual ~Action() {}
    virtual void run() = 0;
};

template < typename Command, typename DelayPolicy = ActionDelayPolicy_NoWait >
class Action : public DelayPolicy, public Command
{
public:
   virtual run() {
      DelayPolicy::wait(); // inherit wait from DelayPolicy
      Command::execute();  // inherit command to execute
   }
};

// Real executed code can be written once (for each action to execute)
class CommandSalute
{
public:
   void execute() { std::cout << "Hi!" << std::endl; }
};

class CommandSmile
{
public:
   void execute() { std::cout << ":)" << std::endl; }
};

// And waiting behaviors can be defined separatedly:
class ActionDelayPolicy_NoWait
{
public:
   void wait() const {}
};

// Note that as Action inherits from the policy, the public methods (if required)
// will be publicly available at the place of instantiation
class ActionDelayPolicy_WaitSeconds
{
public:
   ActionDelayPolicy_WaitSeconds() : seconds_( 0 ) {}
   void wait() const { sleep( seconds_ ); }
   void wait_period( int seconds ) { seconds_ = seconds; }
   int wait_period() const { return seconds_; }
private:
   int seconds_;
};

// Polimorphically execute the action
void execute_action( Action& action )
{
   action.run();
}

// Now the usage:
int main()
{
   Action< CommandSalute > salute_now;
   execute_action( salute_now );

   Action< CommandSmile, ActionDelayPolicy_WaitSeconds > smile_later;
   smile_later.wait_period( 100 ); // Accessible from the wait policy through inheritance
   execute_action( smile_later );
}

The use of inheritance allows public methods from the policy implementations to be accessible through the template instantiation. This disallows the use of aggregation for combining the policies as no new function members could be pushed into the class interface. In the example, the template depends on the policy having a wait() method, which is common to all waiting policies. Now waiting for a time period needs a fixed period time that is set through the period() public method.

In the example, the NoWait policy is just a particular example of the WaitSeconds policy with the period set to 0. This was intentional to mark that the policy interface does not need to be the same. Another waiting policy implementation could be waiting on a number of milliseconds, clock ticks, or until some external event, by providing a class that registers as a callback for the given event.

If you don't need polymorphism you can take out from the example the base class and the virtual methods altogether. While this may seem overly complex for the current example, you can decide on adding other policies to the mix.

While adding new orthogonal behaviors would imply an exponential growth in the number of classes if plain inheritance is used (with polymorphism), with this approach you can just implement each different part separately and glue it together in the Action template.

For example, you could make your action periodic and add an exit policy that determine when to exit the periodic loop. First options that come to mind are LoopPolicy_NRuns and LoopPolicy_TimeSpan, LoopPolicy_Until. This policy method ( exit() in my case ) is called once for each loop. The first implementation counts the number of times it has been called an exits after a fixed number (fixed by the user, as period was fixed in the example above). The second implementation would periodically run the process for a given time period, while the last one will run this process until a given time (clock).

If you are still following me up to here, I would indeed make some changes. The first one is that instead of using a template parameter Command that implements a method execute() I would use functors and probably a templated constructor that takes the command to execute as parameter. The rationale is that this will make it much more extensible in combination with other libraries as boost::bind or boost::lambda, since in that case commands could be bound at the point of instantiation to any free function, functor, or member method of a class.

Now I have to go, but if you are interested I can try posting a modified version.