Polymorphism and Dynamic Casting

Question

So I'm working on a text-based RPG, and I've run into an issue. I am currently working on equipping weapons from the character's inventory. I am trying to make it so that my program can tell if the item they want to equip is of class Weapon or not. Here is the clip of relevant code:

 Item tempChosenWeapon = myInventory.chooseItem();
cout << tempChosenWeapon.getName() << endl;
Item *chosenWeapon = &tempChosenWeapon;
cout << chosenWeapon->getName() << endl;//THE CODE WORKS UP TO HERE

Weapon *maybeWeapon = dynamic_cast<Weapon*>(chosenWeapon);
cout << maybeWeapon->getName() << endl;

Now, Weapon is a child class of Item, which is why I am using dynamic cast -- in an attempt to change chosenWeapon, which is of type Item, to type Weapon in order to compare the two classes. (I am using these cout<<s in or to test whether or not calling a function from these objects works).

My program compiles, and everything runs fine until we come to maybeWeapon->getName(), in which the program crashes. I've researched quite a bit, but I just don't understand what I am doing wrong. Any answer or alternative suggestion is much appreciated! Thanks!

casting is normally a design flaw. You could solve this by using an enum that has the different classes of items you can have then then have a `virtual getItemType()` function that returns the type. This way you don't have to cast and deal with all of the pitfalls. — NathanOliver, Aug 22 '18 at 19:50
I think you don't need to know the exact class. I think that all you want to know is whether the item can be equipped. So, one alternative approach is to have all objects inherit an `Equip()` method from `Item`. Defining this as a virtual function allows `Weapon` objects to respond in one way, and (say) `Talisman` objects to do something else, while the base class `Equip()` does nothing (or prints a hint/error for the user). — Tim Randall, Aug 22 '18 at 19:56
Cast fails, you get a `nullptr`, you dereference the return value without checking, bad things ensue. — Jesper Juhl, Aug 22 '18 at 20:10
Consider adding a polymorphic method "is_weapon()", that returns bool. Derived classes implement their own answer uniquely, and in a single line of code. To reduce effort, all the non-weapons could use the base-class defined "is_weapon()" which returns false. Perhaps your weapon's have grades (tank, etc.) is_weapon() could return the grade enum. non-weapons would be valid, but clearly not a weapon. So many way to avoid dynamic-casting. I agree with Nathan, casting is a design flaw. — 2785528, Aug 22 '18 at 20:17

Christophe · Accepted Answer · 2018-08-22T20:25:52.060

The problem

The problem is that you try to make a dynamic cast to a Weapon but in reality the object pointed to is a true copy constructed Item and not a subclass. This is results in a nullptr and UB when you dereference it !

Why ?

Let's suppose that you have only Weapon objects in your inventory. The first instruction in your snippet is the root of your evil:

    Item tempChosenWeapon = myInventory.chooseItem();

This is statement is a copy construction of an Item object. If the source object was a Weapon, it will be sliced.

Later you take a pointer to this object:

    Item *chosenWeapon = &tempChosenWeapon;

But this Item* doesn't point to a Weapon object as you think. It points to a real crude Item object ! So when you do the dynamic cast here:

    Weapon *maybeWeapon = dynamic_cast<Weapon*>(chosenWeapon);

the code will find out that choosenWeapon is not a Weapon*, and the result of dynamic_cast will be a nullptr. Until now it's not necessarily a catastrophe. But when you then derefence this pointer you get UB:

    maybeWeapon->getName()     // OUCH !!!!!!

Solution

Checking if the dynamic_cast was successful (i.e. result not nullptr) is a protection against the crash, but will not solve your root problem.

It is even possible that the problem is even deeper than expected: what type does the myInventory.chooseItem() return in reality ? Is it a plain Item ? Then you might have the slicing problem already in the inventory !

If you want to use polymorphism:

you have to work with pointers (preferably smart pointers) or with references, in order not to loose the original type of an object, like it happened here.
If you need to copy polymorphic objects, you can't just use an assignment with an Item: you'd need to invoke a polymorphic clone() function and ensure that the target of this cloning has a compatible type.

To start with a solution, it's something like this:

Item* chosenWeapon = myInventory.chooseItem();  // refactor choosItem() to return a pointer.
cout << chosenWeapon->getName() << endl; 
Weapon *maybeWeapon = dynamic_cast<Weapon*>(chosenWeapon);
if (maybeWeapon) 
    cout << maybeWeapon->getName() << endl;
else cout << "Oops the chosen item was not a weapon" <<endl;

If this still not work, then your inventory container would be flawed. In this case, look at this question before opening a separate question with the code of your container

The slice is indeed the root of the problem, which other answer miss. — Jeffrey, Aug 22 '18 at 20:24
Thank you very much! One thing I don't understand here, however: why do you have the if statement as if (maybeWeapon); it doesn't seem like it should be a true/false statement here... would you mind elaborating? Thanks! — Little Boy Blue, Aug 23 '18 at 01:00
@TheMachoMuchacho the if statement takes for true every expression that is not null. So if(maybeWeapon) will have exactly the same behavior than if(maybeWeapon!=nullptr). — Christophe, Aug 23 '18 at 05:21
Ah, I see! Well thank you so much for your time and your help! — Little Boy Blue, Aug 23 '18 at 13:04

score 3 · Answer 2 · answered Aug 22 '18 at 19:50

dynamic_cast will return nullptr if the pointer cast cannot be performed (for reference casts it will throw an exception), so your code should read something like:

Weapon *maybeWeapon = dynamic_cast<Weapon*>(chosenWeapon);
if ( maybeWeapon  ) {
   cout << maybeWeapon->getName() << endl;
else {
   // it's not a weapon
}

If you don't perform that test, and try to dereference the pointer containing nullptr, you are off in Undefined Behaviour Land.

score 0 · Answer 3 · answered Aug 22 '18 at 20:18

Item tempChosenWeapon = myInventory.chooseItem();

this is an Item. Not a type descended from Item. It is an Item.

Values in C++ have known types.

cout << tempChosenWeapon.getName() << endl;

all good, but please stop using namespace std;

Item *chosenWeapon = &tempChosenWeapon;

This is a pointer to an Item. I can prove it is not polymorphic, because it is a pointer to a instance of type Item. The compiler can probably prove it to.

cout << chosenWeapon->getName() << endl;//THE CODE WORKS UP TO HERE

ok, this repeats the previous call.

Weapon *maybeWeapon = dynamic_cast<Weapon*>(chosenWeapon);

This deterministically returns nullptr. chosenWeapon is an Item* that we know points to an Item, and an Item is not a Weapon.

cout << maybeWeapon->getName() << endl;

this dereferences nullptr.

There are a number of ways to handle polymorphism in C++. But you have to think about it.

First, do you want value semantics? Value sematnics means that a copy of something is a copy of it. Things don't refer to other things; they are those things.

You can do value semantics with polymorphic values, but it takes a bit of work. You write two classes; the value wrapper, and the internal pImpl.

The internal pImpl has a std::unique_ptr<Impl> Impl->clone() const method, and the value wrapper calls it when you copy it.

You write your interface like this:

template<class D>
struct clonable {
  std::unique_ptr<D> clone() const = 0;
};
struct ITarget;
struct IItem:clonable<IItem> {
  virtual std::string get_name() const = 0;
  virtual bool can_equip( ITarget const& ) const = 0;
  ~virtual IItem() {}
};
struct Target;
struct Item {
  using Impl = IItem;
  explicit operator bool() const { return (bool)pImpl; }
  IItem* get_impl() { return pImpl.get(); }
  IItem const* get_impl() const { return pImpl.get(); }
  template<class D>
  D copy_and_downcast() const& {
    auto* ptr = dynamic_cast<typename D::Impl const*>( pImpl.get() );
    if (!ptr) return {};
    return D(ptr->clone());
  }
  template<class D>
  D copy_and_downcast() && {
    auto* ptr = dynamic_cast<typename D::Impl*>( pImpl.get() );
    if (!ptr) return {};
    pImpl.release();
    return D(std::unique_ptr<typename D::Impl>(ptr));
  }
  std::string get_name() const {
    if (!*this) return {};
    return pImpl->get_name();
  }
  bool can_equip(Target const& target)const{
    if (!*this) return false;
    if (!target) return false;
    return pImpl->can_equip( *target.get_impl() );
  }
  Item() = default;
  Item(Item&&) = default;
  Item& operator=(Item&&) = default;
  Item(std::unique_ptr<IItem> o):pImpl(std::move(o)) {}
  Item(Item const& o):
    Item( o?Item(o.pImpl->clone()):Item{} )
  {}
  Item& operator=( Item const& o ) {
    Item tmp(o);
    std::swap(pImpl, tmp.pImpl);
    return *this;
  }
private:
  std::unique_ptr<IItem> pImpl;
};

which probably has bugs and is maybe too complex for you.

Second, you can go with reference semantics.

In this case, you want to return shared_ptr<const T> or shared_ptr<T> from your data. Or you can go half way and return a unique_ptr<T> copy from your chooseItem functions.

Reference semantics is really hard to get right. But you do get to use dynamic_cast or dynamic_pointer_cast directly.

std::shared_ptr<Item> chosenWeapon = myInventory.chooseItem();
if (!chosenWeapon) return;
std::cout << chosenWeapon->getName() << std::endl;

auto maybeWeapon = dynamic_pointer_cast<Weapon>(chosenWeapon);
if (maybeWeapon)
  std::cout << maybeWeapon->getName() << std::endl;
else
  std::cout << "Not a weapon" << std::endl;

Stephan Lechner · Answer 4 · 2018-08-22T20:36:09.470

You cannot cast an object of type Item to an object of a subclass of Item. Note that with Item tempChosenWeapon = myInventory.chooseItem(), you will get an Item-object, even if chooseItem might return a Weapon-object. This is called "slicing" and cuts out an Item-subobject of any Weapon-object. Note that variables that are not references or pointers are not polymorphic:

struct A {
    int a = 0;
    virtual void print() const {
        std::cout << "a:" << a << std::endl;
    }
};

struct B : public A {
    int b = 1;
    void print() const override {
        std::cout << "a:" << a << "; b:" << b << std::endl;
    }
};

B b;

A get_b() {  // will slice b;
    return b;
}

A& getRefTo_b() {  // reference to b; polymorphic
    return b;
}

A* getPointerTo_b() {  // pointer to b; polymorphic.
    return &b;
}

int main() {

    A a1 = get_b(); // copy of A-subobject of b; not polymorphic
    a1.print();
    // a:0

    A a2 = getRefTo_b();  // copy of A-subobject of referenced b-object; not polymorphic
    a2.print();
    // a:0

    A &a3 = getRefTo_b(); // storing reference to b-object; polymorphic
    a3.print();
    // a:0; b:1

    A *a4 = getPointerTo_b();  // pointer to b-object; polymorphic
    a4->print();
    // a:0; b:1

    B* b1 = dynamic_cast<B*>(&a1);  // fails (nullptr); a1 is not a B
    B* b2 = dynamic_cast<B*>(&a2);  // fails (nullptr); a2 is not a B
    B* b3 = dynamic_cast<B*>(&a3);  // OK; a3 refers to a B-object
    B* b4 = dynamic_cast<B*>(a4);   // OK; a4 points to a B-object

    return 0;
}

So your signature should probably be

Item &Inventory::chooseItem() {
   static Weapon weapon;
   ...
   return weapon;
};

int main() {
   Item &myWeapon = myInventory.chooseItem();
   Weapon* w = dynamic_cast<Weapon*>(&myWeapon);
   ...
}

Polymorphism and Dynamic Casting

4 Answers4

The problem

Why ?

Solution