0

I am having trouble using virtual functions with objects declared using automatic storage duration. Here is a reproducible scenario:

#include <iostream>

class A {
    public:
        A() {}
        virtual ~A() {}
        virtual void printClassName() {
            std::cout << "A" << std::endl;
        }
};

class B : public A {
    public:
        B() : A() {}
        ~B() {}
        void printClassName() {
            std::cout << "B" << std::endl;
        }
};

class Test {
    private:
        A item;

    public:
        Test() {}
        ~Test() {}
        void setItem(A item) {
            this->item = item;
        }
        A getItem() {
            return this->item;
        }
};

int main() {
    Test t;
    B item;

    t.setItem(item);
    t.getItem().printClassName();

    return 0;
}

This prints "A", while I would expect it to print B. I'm curious as to why.

Thank you in advance!

Chad Johnson
  • 21,215
  • 34
  • 109
  • 207
  • Make sure that all non-leaf base classes are abstract. Otherwise it can be really hard to work with classes. – Kerrek SB Jun 02 '16 at 14:22
  • 1
    This might be worth a read: http://stackoverflow.com/questions/274626/what-is-object-slicing – NPE Jun 02 '16 at 14:25
  • @KerrekSB I tried making `printClassName()` pure virtual, but this results in a compilation error on line 21 then: `error: field type 'A' is an abstract class`. – Chad Johnson Jun 02 '16 at 14:26
  • 2
    @ChadJohnson: Correct. And now you have found your problem. – Kerrek SB Jun 02 '16 at 14:26
  • @KerrekSB Not sure what you are suggesting with "make sure that all non-leaf base classes are abstract," as doing so results in an error. – Chad Johnson Jun 02 '16 at 14:27
  • 4
    @ChadJohnson: No, you got it the wrong way round: *Your code* suffers from a misunderstanding. Making base classes abstract *exposes* that misunderstanding. It is therefore a useful practice that prevents accidents like yours (which can result from as much as single character typo). – Kerrek SB Jun 02 '16 at 14:28

3 Answers3

8

You're slicing here:

void setItem(A item) {
    this->item = item;
}

The B part is sliced off when you pass item into setItem(), so you're losing the base part of this. If A were abstract, this would be a more obvious error (since you can't store an abstract class by value).

Instead, you'll want to to store the item as a pointer.

Community
  • 1
  • 1
Barry
  • 286,269
  • 29
  • 621
  • 977
4

This phenomenon is called object slicing.

Basically, when you are passing item (of type B) to setItem (which takes a A), you are losing every information of B, because a B cannot be stored in a A.

So, when you call printClassName, it only knows about that function from A, so it calls it from A.

One possible solution is to pass pointers to the objects, instead of the raw data. This will call printClassName of B in your example, because no slicing occurs.

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
3

Just to make it clear, to avoid object slicing in this case you should change void setItem(A item) to void setItem(A& item) and A getItem() to A& getItem().

This (passing/returning by reference) preserves dynamic types and is almost certainly what you would intend in this case

EDIT: I overlooked the fact that the object you're storing is also of type A. This cannot work. You need to have it as a pointer in order for it to be able to have a different dynamic type from its static type (or reference, but then it would have to be initialised in the constructor).

So to change to using a pointer, you could try to change A item in Test to A* item. Then set/get become:

    void setItem(A& item) {
        this->item = &item;
    }
    A& getItem() {
        return *(this->item);
    }

This gives you the desired output. This is OK if you're just testing polymorphic behaviour, but would need to be considered very careful for an actual design (for example you may want to consider whether there should be shared ownership using std::shared_ptr).

Smeeheey
  • 9,906
  • 23
  • 39