Advice for best approach at extending class capability

Question

I want to extend a class to include extra data and capabilities (I want polymorphic behavior). It seemed obvious to use inheritance and multiple inheritance.

Having read various posts that inheritance (and especially multiple inheritance) can be problematic, I've begun looking into other options:

• Put all data and functions in one class and not use inheritance
• Composite pattern
• mixin

Is there a suggested approach for the following inheritance example? Is this a case where inheritance is reasonable? (but I don't like having to put default functions in the base-class)

#include <iostream>

//================================
class B {

public:
  virtual ~B() { }

  void setVal(int val) { val_ = val; }

  // I'd rather not have these at base class level but want to use
  // polymorphism on type B:

  virtual void setColor(int val)  { std::cout << "setColor not implemented" << std::endl; }
  virtual void setLength(int val) { std::cout << "setLength not implemented" << std::endl; }

private:
  int val_;

};

//================================
class D1 : virtual public B { 

public:
  void setColor(int color) { 
      std::cout << "D1::setColor to " << color << std::endl;
      color_ = color; 
    }

private:
  int color_;

};

//================================
class D2 : virtual public B {

public:
  void setLength(int length) { 
      std::cout << "D2::setLength to " << length << std::endl;
      length_ = length; 
  }

private:
  int length_;

};

//================================
// multi-inheritance diamond - have fiddled with mixin
// but haven't solved using type B polymorphically with mixins
class M1 : public D1, public D2 {

};


//================================
int main() { 

  B* d1 = new D1;

  d1->setVal(3);
  d1->setColor(1);


  B* m1 = new M1;

  m1->setVal(4);
  m1->setLength(2);
  m1->setColor(4);

  return 0;
}

Answer 1

Suspected problems with the original example code

There are a number of issues with your example.

In the first place, you don't have to supply function bodies in the base class. Use pure virtual functions instead.

Secondly, both your classes D1 and D2 miss functionality, so they should be abstract (which will prevent you from creating deprived objects from them). This second issue will become clear if you indeed use pure virtual functions for your base class. The compiler will start to issue warnings then.

Instantiating D1 as you do with new D1, is bad design, because D1 has no truly functional implementation of the setLength method, even if you give it a 'dummy' body. Giving it a 'dummy' body (one that doesn't do anything useful) so masks your design error.

So your remark (but I don't like having to put default functions in the base-class) testifies of a proper intuition. Having to do that signals flawed design. A D1 object cannot understand setLength, while its inherited public interface promises it can.

And: There's nothing wrong with multiple inheritance, if used correctly. It is very powerful and elegant. But you have to use it where appropiate. D1 and D2 are partial implementations of B, so abstract, and inheriting from both will indeed give you a complete implementation, so concrete.

Maybe a good rule to start with is: Use multiple inheritance only if you see a compelling need for it. But if you do, as said, it's very useful. It can prevent quite some ugly asymmetry and code duplication, compared to e.g. a language like Java, that has banned it.

I am not a tree doctor. When I use a chainsaw I endanger my leg. But that is not to say chainsaws ain't useful.

Where to put the dummy: Nowhere please, do not disinherit...

[EDIT after first comment of OP]

If you derive a class D1 from B that would print 'setLength not implemented' if you call its setLength method, how should the caller react? It shouldn't have called it in the first place, which the caller could have known if D1 did not derive from a B that has this methods, pure virtual or not. Then it would have been clear that it just doesn't support this method. Having the B baseclass makes D1 feel at home in a polymorphic datastructure who'se element type, B* or B&, promises its users that its objects properly support getLength, which they don't.

While this is not the case in your example (but maybe you left things out), there may of course be a good reason to derive D1 and D2 from B. B could hold a part of the eventual interface or implementation of its derived classes that both D1 an D2 need.

Suppose B had a method setAny (key, value) (setting a value in a dictionary), which D1 and D2 both use, D1 calls it in setColor and D2 calls it in setLength. In that case use of a common base class is justified. In that case B should not have virtual methods setColor or setLength at all, neither dummies nor pure. You should just have a setColor in your D1 class and a setLength in your D2 class, but neither of both in your B class.

There's a basic rule in Object Oriented Design:

Do not disinherit

By introducing the concept of a "method that's not applicable" in a concrete class that's just what you're doing. Now rules like this aren't dogma's. But violating this rule almost always points to a design flaw.

All B's in one datastructure is only useful to have them do a trick that they all understand...

[EDIT2 after second coment of OP]

OP wants to have a map that can hold objects of any class derived from B.

This is exactly where the problem starts. To find out how to store pointers and references to our objects, we have to ask: what is the storage used for. If a map, say mapB is used to store pointers to B, there must be some point in that. With data storage the fun is in retrieving the data and doing something useful with it.

Let's make this a bit simpler by working with lists from everyday life. Suppose I have a personList of say 1000 persons, each with their fullName and phoneNumber. And now say I have a problem with the kitchen sink. I could in fact read through the list, call every single Person on it and ask: can you repair my kitchen sink. In other words: do you support the repairKitchenSink method. Or: are you by any chance an instance of class Plumber (are you a Plumber). But then I spend quite some time calling, and maybe after 500 calls or so, I'll be lucky.

Now all 1000 persons on my personList do support the talkToMe method. So whenever I feel lonely I can call any person from that list and invokate that Person's talkToMe method. But they should not all have a repairKitchenSink method, even not a pure virtual or a dummy variation that does something else, because if I would call this method for a person of class Burglar, he'd probably respond to the call, but in an unexpected way.

So class Person shouldn't contain a method repairKitchenSink, even not a pure virtual one. Because it should never called as part of iteration of personList. It should be called when iterating plumberList. This lists only holds objects that support the repairKitchenSink method.

Use pure virtual functions only where appropriate

They may support it in different ways though. In other words, in class Plumber, method repairKitchenSink can e.g. be pure virtual. There may e.g. be 2 derived classes, PvcPlumber and CopperPlumber. CopperPlumber would implement (code) the repairKitchenSink method by calling lightFlame, followed by a call to solderDrainToSink whereas PvcPlumber would implement it as successive calls to applyGlueToPvcTube and glueTubeToSinkOutlet. But both plumber subclasses implement repairKitchenSink, only in different ways. This and only this justifies having the pure virtual function repairKitchenSink in their base class Plumber. Now of course a class may be derived from Plumber that doesn't implement that method, say class WannabePlumber. But since it will be abstract, you cannot instantiate objects from it, which is good, unless you want wet feet.

There may be many different subclasses of Person. They e.g. represent different professions, or different political preferences, or different religions. If a Person is a Democrat Budhist Plumber, than he (M/F) may be in a derived class that inherits from classes Democrat, Budhist and Plumber. Using inheritance or even typing for something so volatile as political preferences or religious beliefs, or even profession and the endless amount of combinations of those, would not be handy in practice, but it's just an example. In reality profession, religion and politicalPreference would probably be attributes. But that doesn't change the point that matters here. IF something is of a class does not support a certain operation, THEN it shouldn't be in a datastructure that suggests it does.

By, besides personList, having plumberList, animistList and democratList, you're sure to call a person that understands your call to method inviteBillToPlayInMyJazzBand, or worshipTheTreeInMyBackyard.

Lists don't contain objects, they only contain pointers or references to objects. So there's nothing wrong with our Democratic Budhist Plumber being contained in personList, democratList, budhistList and plumberList. Lists are like database indexes. The don't contain the records, they just refer to them. You can have many indexes on one table, and you should, because indexes are small and make your database fast.

The same holds for polymorphic datastructures. At the moment that even personList, democratList, budhistList and plumberList become so large that you're running out of memory, the solution is generally NOT to only have a personList. Because then you exchange your memory problem for a perfomance problem and a code complexity problem that, in general, is far worse.

So, back to your comment: You say you want all your derived classes to be in a list of B's. Fine, but still the interface of a B should only contain methods that are implemented for everything in the list, so no dummy methods. That would be like going through the library and going through all books, in search for one that supports the teachMeAboutTheLovelifeOfGoldfishes method.

To be honest, in telling you all this, I've been committing a capital sin. I've been selling general truths. But in software design these don't exist. I've been trying to sell them to you because I've been teaching OO design for some 30 years now, and I think I recognize the point where your stuck. But to every rule there are many exceptions. Still, if I've properly fathomed your problem, in this case I think you should go for separate datastructures, each holding only references or pointers to objects that really can do trick that you were after when you iterated through that particular datastructure.

A point is a square circle

Part of the confusion in properly using polymorphic datastructures (datastructures holding pointers or references to different object types) comes for the world of relational databases. RDB's work with tables of flat records, each record having the same fields. Since some fields may not apply, something called 'constraint' was invented. In C++ class Point would contain field x and y. Class Circle could inherit from it and additionally contain field 'radius'. Class Square could also inherit from Point, but contain field 'side' in addition to x and y. In the RDB world constraints, not fields, are inherited. So a Circle would have constraint radius == 0. And a Square would have constraint side == 0. And a Point would inherit both constraints, so it would meet the conditions for both being a square and a circle: A point is a square circle, which in mathematics indeed is the case. Note that the constraint inheritance hierarchy is 'upside down', compared to C++. Which can be confusing.

What doesn't help either is the generally held belief that inheritance goes hand in hand with specialization. While this is often the case it isn't always. In many cases in C++ inheritance is extension rather than specialization. The two often coincide, but the Point, Square, Circle example shows that this isn't a general truth.

If inheritance is used, in C++ Circle should derive from Point, since it has extra fields. But a Circle certainly isn't a special type of Point, it's the other way round. In many practical libraries, by the way, Circle will contain an object of class Point, holding x and y, rather than inherit from it, bypassing the whole problem.

Welcome to the world of design choices

What you bumped into is a real design choice, and an important one. Thinking very carefully about things like this, as you are doing, and trying them all in practice, including the allegedly 'wrong' ones, will make you a programmer, rather than a coder.

Answer 2

Let me first state that what you are trying to do is a design smell: Most probably what you are actually trying to achieve could be achieved in a better way. Unfortunately we can't know what it is you actually want to achieve since you only told us how you want to achieve it.

But anyway, your implementation is bad, as the methods report "not implemented" to the users of the program, rather than to the caller. There is no way for the caller to react on the method not doing what is intended. Even worse, you don't even output it to the error stream, but to the regular output stream, so if you use that class in any program that produces regular output, that output will be interrupted by your error message, possibly confusing a program further on in a pipeline).

Here's a better way to do it:

#include <iostream>
#include <cstdlib>   // for EXIT_FAILURE

//================================
class B {

public:
  virtual ~B() { }

  void setVal(int val) { val_ = val; }

  // note: No implementation of methods not making sense to a B    
private:
  int val_;

};

//================================
class D1 : virtual public B { 

public:
  void setColor(int color) { 
      std::cout << "D1::setColor to " << color << std::endl;
      color_ = color; 
    }

private:
  int color_;

};

//================================
class D2 : virtual public B {

public:
  void setLength(int length) { 
      std::cout << "D2::setLength to " << length << std::endl;
      length_ = length; 
  }

private:
  int length_;

};

class M1 : public virtual D1, public virtual D2 {

};


//================================
int main() { 

  B* d1 = new D1;

  p->setVal(3);
  if (D1* p = dynamic_cast<D1*>(d1))
  {
    p->setColor(1);
  }
  else
  {
    // note: Use std::cerr, not std::cout, for error messages
    std::cerr << "Oops, this wasn't a D1!\n";
    // Since this should not have happened to begin with,
    // better exit immediately; *reporting* the failure
    return EXIT_FAILURE;
  }

  B* m1 = new M1;

  m1->setVal(4);
  if (D2* p = dynamic_cast<D2*>(m1))
  {
    p->setLength(2);
  }
  else
  {
    // note: Use std::cerr, not std::cout, for error messages
    std::cerr << "Oops, this wasn't a D1!\n";
    // Since this should not have happened to begin with,
    // better exit immediately; *reporting* the failure
    return EXIT_FAILURE;
  }
  if (D1* p = dynamic_cast<D1*>(m1))
  {
    p->setColor(4);
  }
  else
  {
    // note: Use std::cerr, not std::cout, for error messages
    std::cerr << "Oops, this wasn't a D1!\n";
    // Since this should not have happened to begin with,
    // better exit immediately; *reporting* the failure
    return EXIT_FAILURE;
  }

  return 0;
}

Alternatively, you could make use of the fact that your methods share some uniformity, and use a common method to set all:

#include <iostream>
#include <stdexcept> // for std::logic_error
#include <cstdlib>
#include <string>

enum properties { propValue, propColour, propLength };

std::string property_name(property p)
{
  switch(p)
  {
  case propValue: return "Value";
  case propColour: return "Colour";
  case propLength: return "Length";
  default: return "<invalid property>";
  }
}

class B
{
public:
  virtual ~B() {}

  // allow the caller to determine which properties are supported
  virtual bool supportsProperty(property p)
  {
    return p == propValue;
  }
  void setProperty(property p, int v)
  {
    bool succeeded = do_set_property(p,v);
    // report problems to the _caller_
    if (!succeeded)
      throw std::logic_error(property_name(p)+" not supported.");
  }
private:
  virtual bool do_set_property(property p)
  {
    if (p == propValue)
    {
      value = v;
      return true;
    }
    else
      return false;
  }

  int value;
};

class D1: public virtual B
{
public:
  virtual bool supportsProperty(property p)
  {
    return p == propColour || B::supportsProperty(p);
  }
private:
  virtual bool do_set_property(property p, int v)
  {
    if (p == propColour)
    {
      colour = v;
      return true;
    }
    else
      return B::do_set_property(p, v);
  }

  int colour;
};

class D2: public virtual B
{
public:
  virtual bool supportsProperty(property p)
  {
    return p == propLength || B::supportsProperty(p);
  }
private:
  virtual bool do_set_property(property p, int v)
  {
    if (p == propLength)
    {
      length = v;
      return true;
    }
    else
      return B::do_set_property(p, v);
  }

  int length;
};

class M1: public virtual D1, public virtual D2
{
public:
  virtual bool supportsProperty(property p)
  {
    return D1::supportsProperty(p) || D2::supportsProperty(p);
  }
private:
  bool do_set_property(property p, int v)
  {
    return D1::do_set_property(p, v) || D2::do_set_property(p, v);
  }
};