1

I've stumbled upon a diamond inheritance problem, and I am not sure of the best solution. The following code works and has no diamond problem:

class Element { /* pure virtual functions */ };
class Diode : public Element {};
class Thyristor : public Diode {};

I don't like the public inheritance though, because a Thyristor is not a Diode, it just acts like a Diode often enough that I want to use a lot of the Diode code. I can make it work by using composition rather than inheritance, but that results in duplication of internal data structures between Diode and Thyristor which I don't like. What I would like to do is use private inheritance. If I do that, then Thyristor would also need to inherit publicly from Element:

class Thyristor : public Element, private Diode {};

The potential problem is that I have now created a diamond, as Element is inherited directly and through Diode. Is this a problem if Element is a pure virtual function? If it is, what is the proper way to solve this problem, making changes only to the Thyristor class?

jln
  • 119
  • 1
  • 7
  • 5
    *"composition ... results in duplication of internal data structures between Diode and Thyristor"* Can you show why duplication would be unavoidable in this case? – HolyBlackCat Jul 19 '18 at 21:10
  • 1
    See https://stackoverflow.com/questions/5179854/virtual-inheritance-in-c – Some programmer dude Jul 19 '18 at 21:12
  • It's impossible to say for sure without seeing more of your code, but my gut says that `class Thyristor : public Element, private Diode {};` is unlikely to be a good choice. If `Element` is an interface, then you should definitely have public inheritance from it. As for how `Diode` and `Thyristor`, it doesn't seem that `Thyristor` "is a" `Diode`, so don't inherit. – JMAA Jul 19 '18 at 21:16
  • @JMAA If you need `Diode`'s `Element` component to be the same as `Thyristor`'s `Element` component, then you have to do it this way with `virtual` inheritance. – François Andrieux Jul 19 '18 at 21:17
  • @FrançoisAndrieux sure, but I'm just doubting that three levels of inheritance is necessary or desirable here – JMAA Jul 19 '18 at 21:18
  • 1
    Try to factor out common data structures and functionality from `Diode` and `Thyristor`. e.g. if both need some set of thermal properties that are the same, define a `ThermalProperties` struct outside them and give each `Diode` and `Thyristor` one as a member. – JMAA Jul 19 '18 at 21:20
  • @JMAA I agree it might be the right design, but like you pointed it, we can't be sure yet with what we've seen. I'd also like to add that `private` inheritance never means "is a". It's usually implies "implemented in terms of". It's sometimes fine to inherit privately when something "isn't a". Though I feel it likely is meant to be public inheritance. – François Andrieux Jul 19 '18 at 21:20
  • @FrançoisAndrieux good point, but this is getting towards the reasons why I don't like private inheritance for the majority of possible cases – JMAA Jul 19 '18 at 21:21
  • 5
    Using inheritance merely to remove code duplication is a dubious code smell. I worry that you're creating more problems than you're solving by using inheritance this way. – Drew Dormann Jul 19 '18 at 21:35
  • If you want to remove code duplication, put the code in a function. – Mike Borkland Jul 19 '18 at 22:13

1 Answers1

0

You should surely use aggregation instead of inheritance this case.

Just trace back and ask yourself: why I'm inheriting here? How the thyristors and diodes could be used in the outer code? Quite soon you'll find you don't need inheritance for elements, just aggregate necessary behaviors (number of contacts, conduction behavior, etc.)

Yury Schkatula
  • 5,291
  • 2
  • 18
  • 42