0

DISCLAIMER: Keep reading only if you want to see how it's NOT done. Apparently it's so wrong you will get eye cancer from it when you see it (I'm immune because I'm a noob :)


So I was thinking about runtime polymorphism in C++ for a while and I came up with the following. Consider this

#include <iostream>

struct Animal
{
    virtual ~Animal(){}
    void make_noise() const {return do_make_noise();}
    protected:
    Animal( ){}
    virtual void do_make_noise()const{std::cout << "Meh\n";}
};
struct Cat:public Animal
{
    void do_make_noise()const{ std::cout<< "meow\n";}
};
struct Dog:public Animal
{
    void do_make_noise()const{ std::cout<< "woof\n";}
};

int main()
{
    Cat cat;
    Dog dog;
    Animal a((const Animal&)cat); //every tutorial teaches us that this is bad!!
    a.make_noise();
    a = (const Animal&)dog;
    a.make_noise();
    return 0;
}

As you would expect the output is

Meh
Meh

OK, object slicing is bad :/
But now let me change the Base class slightly:

struct Animal
{
    virtual ~Animal(){}
    void make_noise() const {return ptr_->do_make_noise();}
    protected:
    Animal( ):ptr_(this){}
    Animal* ptr_;
    virtual void do_make_noise()const{std::cout << "Meh\n";}
};

Then the result is

meow
woof

Haha. Screw object slicing ...
What do you think of it? It seems to me that it has many advantages to be able to copy by value and still get the polymorphic behaviour. Is there a reason why people do not use this? Or am I just reinventing the wheel here?

  • 9
    That sounds like a recipe for invalid-pointer-dereference disaster. – user2357112 Jul 22 '17 at 21:27
  • 7
    Now you have an object A containing a (non-owning) pointer to object B. If object B is destroyed first, you have a problem. – melpomene Jul 22 '17 at 21:27
  • 3
    If your subclasses have any member variables, slicing will still screw you over. – cHao Jul 22 '17 at 21:30
  • @cHao I disagree. Try it out it also works if the subclass has members – maze-cooperation Jul 22 '17 at 21:36
  • 2
    Don't think you gain much relative to the work you'll have to put into this to get it stable. You have a Rule of Three violation to fix and you will always have the mis-matched lifetime problem discussed above. Plus all of the descendants of Animal` are carrying around `ptr_` and will never use it. May I recommend using a `std::unique_ptr` instead? – user4581301 Jul 22 '17 at 21:39
  • @melpomene: yes but isn't that true for any class that stores a pointer to a base class. Just consider when you have a memory buffer and you destroy it while another class uses is, then of course you have a problem – maze-cooperation Jul 22 '17 at 21:40
  • I don't see the gain, using (smart)pointer/reference seems natural. make copy constructor protected to avoid misuses. (You have to check that each virtual call is called via `ptr_` instead of `this`). You add extra cost for harder code to write (+ lifetime issue as mentioned above). – Jarod42 Jul 22 '17 at 21:50
  • Your solution is similar to CRTP: https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern – Brandon Jul 22 '17 at 21:56
  • ok, from your reactions I get that it is a bad idea with copying and lifetime being the main issues. I am new to all this inheritance so maybe could you explain more: e.g. when I declare Animal * a = &cat; don't I also have a mismatched lifetime, because if cat gets out of scope before a then the pointer is invalid? – maze-cooperation Jul 22 '17 at 21:57
  • @Jarod42 what's wrong with the default copy constructor? Could you give an example on how this can be misused? And in what situation could the virtual function be called via this? – maze-cooperation Jul 22 '17 at 22:03
  • The default copy constructor (of snippet 1) allows slicing (the misuse). Making it protected would spot the error in `main`. It should be protected instead of private or deleted as `clone()` would need it. – Jarod42 Jul 22 '17 at 22:18
  • 1
    The lifetime dependency is obvious in `Animal * a = &cat;` but not for your `Animal a = cat;`. – Jarod42 Jul 22 '17 at 22:23
  • In addition, a `clone` function is no longer trivial to write with your code. – Jarod42 Jul 22 '17 at 22:25

1 Answers1

4

Your code as written does not exhibit undefined behaviour and does indeed "block object slicing"; instead it is ridiculously fragile and unmaintainable.

Every Animal now holds an unmanaged pointer to some ur-Animal which it acts as (sometimes itself). Your code violates the meaning of sensible copy and move construction.

In effect, it avoids crashing and undefined behaviour, but only by accident. Minor seeminly innocuous tweaks and your program breaks.

Write a function that returns an animal? Your code breaks, depending on elision. Copy from a local Animal to a reference passed in, then return? Broken. Put them in std vectors? Utter disaster.

Creating value semantics polymorphic types that do not suffer slicing looks a tad like this, but you split the virtual heirarchy from the pImpl owning value type, and the instances own what the pImpl points to.

struct IAnimal{
  virtual void speak() const=0;
  virtual std::unique_ptr<IAnimal> clone() const = 0;
  virtual ~IAnimal(){}
};
struct Animal {
  std::unique_ptr<IAnimal> pImpl;
  void speak() const { if (pImpl) pImpl->speak(); }
  explicit operator bool()const{return (bool)pImpl;}
  Animal(Animal&&)=default;
  Animal& operator=(Animal&&)=default;
  Animal(Animal const&o):
    Animal(o.pImpl?Animal(o.pImpl->clone()):Animal())
  {}
  Animal()=default;
  Animal& operator=(Animal const& o){
    Animal tmp(o);
    swap(pImpl, tmp.pImpl);
    return *this;
  }
  Animal(std::unique_ptr<IAnimal> x):pImpl(std::move(x)){}
};

Now Animal cannot be sliced and can be polymorphic.

struct Dog:Animal{
  struct IDog:IAnimal{
    std::unique_ptr<IAnimal> clone()const final override{ return std::make_unique<IDog>(*this); }
    void speak()const final override { std:cout <<"woof\n"; }
  };
  Dog():Animal(std::make_unique<IDog>()){}
};  
struct Cat:Animal{
  struct ICat:IAnimal{
    std::unique_ptr<IAnimal> clone()const final override{ return std::make_unique<ICat>(*this); }
    void speak()const final override { std:cout <<"meow\n"; }
  };
  Cat():Animal(std::make_unique<ICat>()){}
};

Barring tpyos we get:

Cat cat;
Dog dog;
Animal a((const Animal&)cat); //every tutorial teaches us that this is bad!!
a.speak();
a = (const Animal&)dog;
a.speak();

working.

And my copy/move ctors are sensible, and I work in vectors, and I do not have dangling pointers all over.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524