C++ Best practice pattern of using polymorphic data member

Question

What is the best practice pattern of using polymorphic data member in C++? The simplest one that I know is just using a plain pointer.

For example, if the type of data member is not polymorphic:

class Fruit {
  public:
    int GetWeight() { return 1; }
};

class Food {
  public:
    Food(Fruit f=Fruit()) : m_fruit(f) {}
    void SetFruit(Fruit f) { m_fruit = f; }
    Fruit GetFruit() { return m_fruit; }
    int Test() { return m_fruit.GetWeight(); }
  private:
    Fruit m_fruit;
};
// Use the Food and Fruit classes
Food food1; //global scope
void SomeFunction() {
  Fruit f;
  food1.SetFruit(f);
}
int main() {
  Fruit fruit1, fruit2;
  Food food2(fruit1);
  food2.SetFruit(fruit2);
  food2.SetFruit(Fruit());
  SomeFunction();
  food1.Test();
  food2.Test();
  return 0;
}

It is easy to use the classes in many ways with minimum possibility for errors.

Now if we make Fruit a base class (use polymorphic), this is the simplest/best one I can think of:

class Fruit {
  public:
    virtual int GetWeight() { return 0; }
};

class Apple : public Fruit {
  public:
    virtual int GetWeight() { return 2; }
};

class Banana : public Fruit {
  public:
    virtual int GetWeight() { return 3; }
};

class Food {
  public:
    Food(Fruit& f=defaultFruit) : m_fruit(&f) {}
    void SetFruit(Fruit& f) { m_fruit = &f; }
    Fruit& GetFruit() { return *m_fruit; }
    int Test() { return m_fruit->GetWeight(); }
  private:
    Fruit* m_fruit;
    static const Fruit m_defaultFruit = Fruit();
};
// Use the Food and Fruit classes
Food food1; //global scope
void SomeFunction() {
  Apple a;
  food1.SetFruit(a);
}

int main() {
  Apple a;
  Banana b;
  Food food2(a);
  food2.SetFruit(b);
  food2.SetFruit(Apple()); //Invalid ?!
  SomeFunction(); //Weakness: the food1.m_fruit points to invalid object now.
  food1.Test();   //Can the compiler detect this error? Or, can we do something to assert m_fruit in Food::Test()?
  food2.Test();
  return 0;
}

Is there any better way of doing this? I try to keep it simple not using any new/delete statement. (Note: must be able to have default value)

UPDATE: Lets consider a more practical example:

class Personality {
public:
  void Action();
};
class Extrovert : public Personality {
  ..implementation..
};
class Introvert : public Personality {
  ..implementation..
};

class Person {
  ...
  void DoSomeAction() { personality->Action(); }
  ...
  Personality* m_personality;
};

The aim is to have persons with different personalities, and to make it easy to add more Personality by subclassing. What is the best way to implement this? Since logically, we only need one object for each type of Personality, it does not make much sense for a Person to copy or assume ownership of Personality object. Could there be any difference approach/design that does not require us to deal with copy/ownership problem?

Answer 1

Your SetFruit() is assigning f's address to m_fruit, not the address of whatever was passed in. Since f is just a function argument, it exists on the stack only until SetFruit() returns. After that, f goes away and its address is no longer valid. Thus, m_fruit is pointing to something that doesn't exist.

So instead, do something like this:

class Food {
  public:
    Food(Fruit* f = &defaultFruit) : m_fruit(f) {}
    void SetFruit(Fruit* f) { m_fruit = f; }
    Fruit* GetFruit() { return m_fruit; }
    int Test() { return m_fruit->GetWeight(); }
  private:
    Fruit* m_fruit;
    static const Fruit m_defaultFruit;
};

Polymorphism requires that you do everything with pointers (or references as certain parameters, but that doesn't apply to your case).

Answer 2

In this case, you're better off with separate default and copy constructors. Also, you shouldn't take the address of something passed in by reference; it could be an object on the stack that will go away soon. In the case of your function:

void SomeFunction() {
  Apple a;
  food1.SetFruit(a);
}

a (and thus &a as well) becomes invalid when the function returns, so food1 would contain an invalid pointer.

Since you're storing a pointer, you should expect the Food to assume ownership of the passed-in Fruit.

class Food {
  public:
    Food() : m_fruit(new Fruit()) {}
    Food(Fruit* f) : m_fruit(f) {}
    void SetFruit(Fruit* f) { if(m_fruit != f) { delete m_fruit; m_fruit = f; } }
    Fruit& GetFruit() { return *m_fruit; }
    const Fruit& GetFruit() const { return *m_fruit; }
    int Test() { return m_fruit->GetWeight(); }
  private:
    Fruit* m_fruit;
};

The other option is for Foodto make a copy of the Fruit it is passed. This is tricky. In the old days, we would define a Fruit::clone() method which subclasses would override:

class Fruit {
  public:
    virtual int GetWeight() { return 0; }
    virtual Fruit* clone() const { return new Fruit(*this); }
};

class Apple : public Fruit {
  public:
    virtual int GetWeight() { return 2; }
    virtual Fruit* clone() const { return new Apple(*this); }
};

class Banana : public Fruit {
  public:
    virtual int GetWeight() { return 3; }
    virtual Fruit* clone() const { return new Banana(*this); }
};

I don't think you can make a virtual constructor in the way you'd need to make this prettier.

Then the Food class changes to:

class Food {
  public:
    Food() : m_fruit(new Fruit()) {}
    Food(const Fruit& f) : m_fruit(f.clone()) {}
    void SetFruit(const Fruit& f) { delete m_fruit; m_fruit = f.clone(); }
    Fruit& GetFruit() { return *m_fruit; }
    const Fruit& GetFruit() const { return *m_fruit; }
    int Test() { return m_fruit->GetWeight(); }
  private:
    Fruit* m_fruit;
};

With this setup, your SomeFunction would work since food1 no longer depends on a's existence.

Answer 3

There isn't a clean and simple way to do it without operator new, because the different objects may take up different amount of space in memory. So Food can't contain a Fruit itself, but only a pointer to Fruit, which in turn would have to be allocated on the heap. So here are some alternatives.

Take a look at the library boost::any. It exists precisely to solve your problem. The down side is that it adds a lot of Boost dependencies, and while it may make your code look simple, it would be harder to understand for anyone not familiar with the library. Underneath, of course, it still does new/delete.

To do those manually, you can make Food accept a pointer to Fruit, and take ownership of it by calling delete in its destructor ~Food() (or it can use a smart pointer like shared_ptr, scoped_ptr, or auto_ptr, to have it deleted automatically). You'd have to clearly document that the passed in value MUST be allocated with new. (The default value will present a special annoyance; you'd have to either check for it, or add a flag, or allocate a copy of that value with operator new as well.)

You may want to be able to take any Fruit as argument and make a copy of it on the heap. That takes more work because you don't know, when making the copy, which Fruit it actually is. One common way to solve it involves adding a virtual method clone() to the Fruit, which derived Fruits have to implement appropriately.