diamond problem - or not

[Agent Smith]

Hello all,

I have been thinking about this and I think I need some help.
I have the following situation:
I'm working on a project that involves using a bunch of different devices, each with it's own functionality, for example:
- UPS
- Card reader
- Database server
- Payout device
- Banknote acceptor device.

Until now my structure was like this:
CDevice, abstract class with methods like Reset, IsEnabled, Initialize, GetLastError, Check... that are common to all devices.
Because a Card reader is a kind of device I chose:
CUpsDeviceDevice, abstract, inherits from CDevice, with methods that define how an UPS works
CCardRederDevice, abstract, inherits from CDevice, with methods that define how a Card reader works,
etc.
Then, for each device, another class inherits from one of the above and actually implements a device, dealing with device specific code for Check, Reset, etc and for the specialized part (ex CCardReaderDevice).
A core object stores all these devices in an array as CDevice* and I have some like GetCardReader() that returns CCardReaderDevice* and so on for each device type.
When I need to work with a device I usually do it like this:
call GetCardReader(), check it by calling Check and GetLastError (inherited from CDevice), and if it IsEnabled do something card reader related by calling CCardReaderDevice functions. But there are also areas in the code where I don't care what type of device I have, all I want is to do Check, GetLastError, Reset, Initialize...only CDevice related (periodical checking of all devices, error reporting, etc.)
At that time this seemed as a good design. But now I have to add a new device, that it is both a payout device and an acceptor (who could have known they would invent such things :D - it recycles the banknotes received into trays that then are used for payment when instructed). My problem is how should I implement this device. It is a single device that acts like two: it needs methods from CDevice and to be able to see it through a CDevice* but also I need it as CPayoutDevice or as CAcceptorDevice. When I call Check, GetLastError and co. from CPayoutDevice or CAcceptorDevice it should report the same state, of the actual device.

- If I implement it as a class inheriting from both CPayoutDevice and CAcceptorDevice then I would have the "dreaded" diamond problem. I could use virtual inheritance when inheriting from CDevice and solve the problem. After all the new device is a payout device and an acceptor device (which in turn are general devices), why not make it so. This would fit with the current design with minimal changes.

- If I change the architecture and choose not to derive each device class from CDevice and instead each implementation will derive from both CDevice and CCardReader for example. But now how can I call CDevice functions when I get a pointer to CCardReader to check it it is enabled, has error or such.

Any ideas are appreciated (also, please consider the impact on the current design, I cannot afford to rewrite everything).
Is the diamond solution such a bad thing?

Later Edit: I also chose this design because if I need to add a new Card Reader device from a different manufacturer all I need is to create a new class that implements CCardReaderDevice and add it to the project. No other code changes are needed.

[doug4]

Your current design sounds really well thought-out. We had a similar situation here, and I understand the agony of an elegant design running into the real world.

A couple of possible solutions.

Instead of inheriting one of the things, say Acceptor Device, you could write the Acceptor Device code as a contained class. Then your CAcceptorDevice and your new class could both contain the AccpetorDevice functionality. This would not allow all AcceptorDevice object to derive from a common base class, but might work, depending on what you are trying to do with it.

Another option is mixin classes (google it). You could write a CAcceptorDevice mixin template and then use the mixin template to wrap both the base CDevice class and the CPayoutDevice class. If you play with it enough, you can derive from both. Maybe something like this: (No guarantees of syntax correctness)

class CAcceptorDeviceBase // Not derived from CDevice;
{
// Functionality here
};

template <class DEVICE>
CAcceptorDeviceTemp : public DEVICE, public CAcceptorDeviceBase
{
// override DEVICE code to also call CAcceptorDeviceBase code as necessary
};

typedef CAcceptorDeviceTemp<CDevice> CAcceptorDevice;
typedef CAccpetorDeviceTemp<CPayoutDevice> CAcceptorPayoutDevice;

This will take some effort to get it to work, and I may not have the mixin correctly chosen, but it might be worth a shot. This method will allow all classes to be derived from CAccpetorDeviceBase.

[clanmjc]

Is the diamond solution such a bad thing?

All things considered I think this is the most cost effective solution based on the info you've provided. I would just document the derives that need to use virtual inheritance for the next guy.

[Galik]

Whereas ending up with a diamond structure can indicate bad design (by trying to create functionality through inappropriate inheritance for example), the diamond structure has its place. And it is particularly suited to input/output style problems where you have two distinct but similar interfaces providing opposing functionality.

So I don't see why not in your case.

[clanmjc]

As Galik said I believe iostream has the diamond structure. Inherits from istream and ostream and both of those inherit from ios_base.

Another way you could re architect (which would probably be a little more work than using virtual inheritance) is not have the specialized interfaces derive from CDevice. This is what I mean.


This is your current hierarchy correct?

CDevice -> CPayoutDevice -> PayoutConcrete
CDevice -> CAcceptorDevice -> AcceptorConcrete

Instead of this decouple the interfaces for the specialized devices from the CDevice. Such that the concrete object inherits from CDevice and CPayoutDevice. What does this buy you? When you have a concrete type that needs to implement the interface of more than 1 specialized type (acceptor, payout) you just need to implement that additional interface.

CDevice -> PayoutConcrete  //multiply inherits
CPayout ->

CDevice    -> AcceptorConcrete   //same
CAcceptor  ->

//And now the double interface.
CDevice    -> AcceptorPayoutConcrete  //1 Device, 1Acceptor 1 Payout
CAcceptor  ->
CPayout    ->

But now how can I call CDevice functions when I get a pointer to CCardReader to check it it is enabled, has error or such.

If the CCardReader pointer points to an object that inherits from CDevice and CCardReader you can just dynamic_cast<CDevice *>(CCardReaderPointer);

[Agent Smith]

@clanmjc: Yes, that is my current hierarchy. I also thought about this but didn't realized that dynamic_cast could solve that issue.

Thanks all for the answers. I now have some directions to follow. I'll try them on a small scale and see where it goes from there.