Design dilemma: extensibility vs simplicity

Question

Here is my problem abstracted to Bird classes. I know that number of Birds will increase on the future and new behaviors might be needed. With 10 Birds first design might not look so simple and lead to lots of duplicated code. On the other hand, second design can be perceived as "class explosion". Which of these two designs would be considered best-practice?

Classic:

#include <iostream>

class Bird
{
  public:
    virtual void Fly() const = 0;
    virtual void Speak() const = 0;
};

class Eagle : public Bird
{
  public:
    virtual void Fly() const
    {
      std::cout << "Eagle shall fly now!" << std::endl;
    }
    virtual void Speak() const
    {
      std::cout << "Eagle speaking!" << std::endl;
    }
};


class Penguin : public Bird
{
  public:
    virtual void Fly() const
    {
      std::cout << "Penguin shall fly now!" << std::endl;
    }
    virtual void Speak() const
    {
      std::cout << "Penquin speaking!" << std::endl;
    }
};

int main()
{
  std::cout << "..." << std::endl;
  Bird* bird = NULL;

  bird = new Eagle();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  bird = new Penguin();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  return 0;
}

"Better?

#include <iostream>
#include <cassert>

class FlyStyle
{
  public:
    virtual void Fly() const = 0;
};

class FlyHigh : public FlyStyle
{
  virtual void Fly() const
  {
    std::cout << "Fly high!" << std::endl;
  }
};

class NoFly : public FlyStyle
{
  virtual void Fly() const
  {
    std::cout << "No fly!" << std::endl;
  }
};

class SpeakStyle
{
  public:
    virtual void Speak() const = 0;
};

class SpeakLoud : public SpeakStyle
{
  virtual void Speak() const
  {
    std::cout << "Speak LAUD!!!!" << std::endl;
  }
};

class NoSpeak : public SpeakStyle
{
  virtual void Speak() const
  {
    std::cout << "No speaking!" << std::endl;
  }
};

class SuperBird
{
  public:
    SuperBird(FlyStyle* fly, SpeakStyle* speak)
      : flystyle(fly),
      speakstyle(speak)
  {
    assert(NULL != flystyle);
    assert(NULL != speakstyle);
  }

    ~SuperBird() { delete flystyle; delete speakstyle;}

    virtual void Fly() const
    {
      flystyle->Fly();
    }
    virtual void Speak() const
    {
      speakstyle->Speak();
    }
  protected:
    FlyStyle* flystyle;
    SpeakStyle* speakstyle;
};

class SuperBirdFactory
{
  public:
    static SuperBird* createEagle()
    {
      return new SuperBird(new FlyHigh(), new SpeakLoud()); 
    }
    static SuperBird* createPenguin()
    {
      return new SuperBird(new NoFly(), new NoSpeak()); 
    }
};

int main()
{
  SuperBird* bird = NULL;

  bird = SuperBirdFactory::createEagle();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  bird = SuperBirdFactory::createPenguin();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  return 0;
}

Answer 1

In my opinion, a better approach (along similar lines) is to avoid abstract base classes and instead use generic "policy" classes. You informally define an interface for each behaviour, and mix them into the class as template parameters. This avoids the need for dynamic memory allocation, and removes the overhead of virtual function calls; everything is resolved at compile time. Your example could be something like this:

#include <iostream>

// Flying styles must have interfaces compatible with this
// struct FlyStyle
// {
//    void Fly() const;
// };

struct FlyHigh
{
  void Fly() const
  {
    std::cout << "Fly high!" << std::endl;
  }
};

struct NoFly
{
  void Fly() const
  {
    std::cout << "No fly!" << std::endl;
  }
};

// Speaking styles must have interfaces compatible with this
// struct SpeakStyle
// {
//     void Speak() const;
// };

struct SpeakLoud
{
  void Speak() const
  {
    std::cout << "Speak LAUD!!!!" << std::endl;
  }
};

struct NoSpeak
{
  void Speak() const
  {
    std::cout << "No speaking!" << std::endl;
  }
};

template <class FlyStyle, class SpeakStyle>
class SuperBird
{
  public:
    void Fly() const
    {
      flystyle.Fly();
    }
    void Speak() const
    {
      speakstyle.Speak();
    }
  private:
    FlyStyle flystyle;
    SpeakStyle speakstyle;
};

typedef SuperBird<FlyHigh, SpeakLoud> Eagle;
typedef SuperBird<NoFly, NoSpeak> Penguin;

int main()
{
  Eagle eagle;
  eagle.Fly();
  eagle.Speak();

  Penguin penguin;
  penguin.Fly();
  penguin.Speak();
}

Answer 2

The fact that you have two completely different outputs from these two examples should be a huge clue as to the answer.

Eagle shall fly now!
Eagle speaking!
Penguin shall fly now!
Penguin speaking!

vs

Fly high!
Speak LAUD!!!!
No fly!
No speaking!

If you want your Penguin class to fly like a Penguin, whatever that means, then you're probably right to go with the first example. If you want all non-flying birds to act the same way as each other when calling fly(), the second is probably better.

That said, I doubt I'd go as far as using a Factory class. I would rename SuperBird as Bird and continue to derive Eagle and Penguin from that, passing the relevant FlyStyle and SpeakStyle to the superclass's constructor, such as

class Penguin : public Bird
{
    public:
        Penguin() : Bird(new NoFly(), new NoSpeak()) {}
}

This is attempting to follow Single Responsibility Principle, by saying "if I want to change the action of all birds which do not fly, I shall change the NoFly class accordingly, but if I want to make a Penguin fly, I will change the Penguin class."

The Factory Pattern is more appropriate to the opposing use case, where your calling code doesn't know or care which type of bird you wish to instantiate but does know enough information for the Factory class to make that decision, such as

bird = BirdFactory::create(CAN_FLY, MAKES_NOISE);
bird.Fly();
bird.Speak();

Answer 3

If there will be different kinds of birds that share a flying or speaking style ("strategy" is what the Gang of Four calls this), then the second design makes sense. If all birds will have unique flying and speaking styles, stick with the first.

If you don't know the answer yet, stick with the first design. In the absence of information, always prefer the simplest solution. It's easier to extend a simple design later than it is to simplify a needlessly extensible one.

Answer 4

Let's see: you've gotten votes for the first and for the second. Hmm...how can I argue with both. Oh, I've got it: both designs must be wrong!

From an abstract viewpoint, you have nothing to justify the Bird/Eagle/Penguin classes as classes at all. In particular, even though you've defined a couple of virtual functions, the functions aren't really doing anything different at all. In all cases, they do exactly the same thing -- print out a string.

As such, you should probably just create birds, and when you create a bird you should specify the "flyingstyle" and "speakingstyle" as arguments to the constructor:

#include <string>
#include <iostream>

class bird { 
    std::string movingstyle;
    std::string speakingstyle;
public:
    bird(std::string const &ms, std::string const &ss) 
        : movingstyle(ms), speakingstyle(ss)
    {}

    void speak() { std::cout << speakingstyle << "\n"; }
    void move() { std::cout << movingstyle << "\n"; }
};

int main() {
    bird birds[2] = {
        bird("Eagle Flying", "Eagle speaking"), 
        bird("Penguin swimming", "Penguin speaking")
    };

    for (int i=0; i<2; i++) {
        birds[i].speak();
        birds[i].move();
    }
    return 0;
}

Reserve separate classes for objects that truly have different behavior, not the same behavior with different values.

I'd also note that I've halfway-repaired a basic flaw in your original hierarchy. By including "fly" in your base class, you've asserted that all birds can fly -- an outright falsehood. Here I've changed that the more abstract concept "move" instead. With this, you can have Penguins that swim, Ostriches that run, and Eagles that fly.

Something similar should probably be done with the "speak". Some birds don't make sounds, so you should consider whether you want something like:

class bird {};

class speaking_bird : public bird {
public: 
    virtual void speak() = 0;    
};

class silent_bird : public bird {};

This correctly models the fact that some birds "speak" and others don't. It also has a fundamental difference in behavior between "speaking bird" and "silent bird" that justifies using inheritance.

Alternatively, you could assert in the design that all birds can speak, but in the implementation say that some birds (that you can I know can't speak) should just never be asked to speak:

class bird {
    bool silent;
public:
    void speak() { if (silent) throw runtime_error("Cannot speak"); }
};

There's a fair basis for saying this is a kludge, but in some cases, it's worth avoiding creating extra levels of inheritance (and such) just to cover obscure corner cases that you'll probably never care about in real use anyway. At the same time, this can lead to littering other code with checks for those corner cases to avoid invoking functions that won't work, and you'd generally rather avoid that as well. As such, you need to look at the specific situation to decide which is the least of the available evils.

Answer 5

Start with the first, refactor to the second.

The interface is the same in both situations. All of the code which is using these classes should not care which method you choose to implement.

You should implement the first, simpler method. When it becomes useful, you should apply other techniques. For example, if you find yourself having a lot of flightless birds you might implement a FlightlessBird class which has the common logic. When speaking becomes too complicated, you might want to break it out into its own object. In other words, implement parts of the second version as they become neccessary.

Your second version is attempting to have maximum flexibility. Attempting to do that has a strange tendency to result in lots of obtuse code and still lack the flexibility that you need. Therefore, in most situations its better to wait until you have a better idea of what you actually need rather then guessing ahead of time.

There is no significant benefit to implementing the flexible version now. There are definite benefits to implementing the simple version now. You only a basic idea of what the code will be required to do later. Attempting to implement that now will only produce a mess.

Answer 6

The first one is definitely better. You should try to use as few classes as will get the job done and also favour lots of small methods over a few big methods. These will make your code easier to read, understand and maintain. I think you could improve on your first one a bit:

#include <iostream>
using namespace std;

class Bird {
    public:
        void print(const char* str) {
            cout << str << endl;
        }
};

class Eagle : public Bird {
    public:
        void fly() {
            print("Eagle shall fly now!");
        }
        void speak() {
            print("Eagle speaking!");
        }
};          

int main() {
    Eagle* eagle = new Eagle();
    eagle->fly();
    eagle->speak();
    return 0;
}

Basically, I'm saying 'An Eagle is a Bird' and would do the same with other birds. I'm keeping the specific parts (eg. what needs to be done for fly() and speak()) in the children classes (such as Eagle) and the general parts (eg. the ability to print to stdout) in the parent class (Bird).

Answer 7

First option is best. It is readable and KISS-compliant. If you ever need to share a flight implementation, you can write a mixin. Also, unrelated, you should look into RAII and get rid of these assert/new/delete.

Answer 8

I really prefer the first as it's simpler. You can always mixin other interfaces later:

class Predator {
   public:
      virtual void attack();
}

class Eagle: public Bird, public Predator {
}

Answer 9

The first example is fine to start with, but you should have in mind where you want it to go where more birds are added, so you don't make changes incompatible with that inevitable change.

First, what are your requirements? I assume from your examples:

1. There will be a common interface for bird types (called "Bird" in the first example)
2. You initially expect all functions to deal with either a bird type, or an eagle type, but you don't know if you will ever need to pass a pointer to a nofly type.
3. You would prefer that if you declare an eagle type at compile time, all the code can be instantiated then, as efficiently as in the first example?

If so, how about:

class Bird
{
  public:
    virtual void Fly() const = 0;
    virtual void Speak() const = 0;
  protected:
    void FlyYes() const {
      std::cout << "No fly!" << std::endl;
    }
    /*
     ...
    */

};

/* final */ class Eagle : public Bird
{
  public:
    void Fly() const
    {
      FlyYes();
    }
    /*
     ...
    */
};


/*
  ...
*/

That's a compromise, but:

• it should all happen at compile time
• it should prevent subtle differences in supposedly equivalent "fly" types
• you can always implement custom functions for birds that fly in a specific way, without needing to create lots more classes for each
• and you can easily move those into the base class if you later want to share them
• if you later realise that all of your birds do fall into some categories, you can create some intermediate partially abstract classes which fall between bird and most leaf classes, eg. a BirdFly class that implements the fly function as I did in Eagle, which Eagle can then inherit from.

Answer 10

This first one is simpler and you might want to use it if you do not need a lot of flexibility. If you ask yourself this question, I think it means you might need flexibility.

In this little example, the first method indeed have fewer classes but in general, if you need a lot of different combinations it might no be the case anymore. Take the second example, if your "SuperBird" is extended be compound of 5 different objects and for each one you have defined 5 abstract types: doing the same thing for the first example would need 3000 classes with a lot of boilerplate repeated in many classes.

See the Gang of Four principle: favor object composition over class inheritance/

Answer 11

It's much more important to be able to change design in future that to select "the correct design" from the start.

The simple way to have this ability is to hide as much as possible.

Are users happy with one class with virtual function? Hide the knowledge that different birds have or have not separate classes.

Are users OK with delegating creation of bird to your module? Hide all constructors of Bird (or make it abstract).

Internally - use whatever is good for current purposes, and change the internal design once it will become inconvenient.

Thus interface may be the following:

class Bird
{
public:
  virtual void Fly()=0;
  virtual void Speak()=0;
};
class BirdFactory
{
public:
  static Bird *CreateEagle();
  ...
};

Everything other should be hidden either as a private stuff of some class or surrounded with other hints about being internal (namespace "detail", directory "internal", etc).