C++ polymorphic pointer cannot invoke member function

Question

I'm trying to set up a simple case to solve a textbook exercise. The code is on IDEone, and repeated below.

The code is a simplified case of trying to store a number of lists of animals, and being able to return any generic animal from one of these animal-specific lists held inside my wrapper queue.

I can add animals just fine and when trying to retrieve a dog, I appear to get a pointer to the dog since I can print its name. However, if I try to call its member function speak(), the code crashes and I can't figure out why.

class Animal {
public:
    virtual void speak() = 0;
    string name;
};

class Dog: public Animal {
public:
    Dog(string n) { this->name=n; }
    void speak() { cout<<name<<" says WOOF!"<<endl; }
};

class AnimalQueue {
    list<Dog> dogs;
    list<Cat> cats; // etc.
public:
    void enqueue(Animal* a) {
        Dog * d = dynamic_cast<Dog*>(a);
        if (d!=nullptr) dogs.push_back(*d);
        else // check for other animals, etc.
    }
    Dog* dequeueDog() {
        Dog * d = &(dogs.front());
        dogs.pop_front();
        return d;
    }
    Animal dequeueAny() {
        // Should return a random animal from any list
    }
};

int main() {
    // Set up
    AnimalQueue q;
    Dog * d;
    d = new Dog("Rex");
    q.enqueue(d);

    // Retrieve Rex
    d = q.dequeueDog();
    cout<<d->name<<endl;  // Prints "Rex"
    d->speak();           // Crashes?!

    return 0;
}

EDIT: Sorry, in cutting down my code, I eliminated the essence of the problem, which is that I should be able to add any sub-class of Animal to my list, and there's a special function called dequeueAny() which should be able to return a random Animal from any list. The code has been edited to include this (as well as my previously omitted nullptr checks when enqueueing.

What's the best way to handle this? Is it to pass in a reference to an existing Animal object? Would that work? ie could, I have:

void dequeueAny(Animal * a) {
    // for example, let's return a Dog
    Dog d = dogs.front();
    dogs.pop_front();
    *a = d;
}

Admittedly, things like dequeueDog() should probably return a Dog by value.

Answer 1

You're returning the address of an element stored (list::front returns a reference and you're taking the address of it) and then popping it (list::pop_front destroys the object):

Dog* dequeueDog() {
    Dog * d = &(dogs.front()); // Take the address of the front object
    dogs.pop_front(); // Destroy the front object
    return d; // Return the address to the deleted object (unsafe state)
}

Accessing memory through that pointer is undefined behavior.

A possible solution could be returning your object by value

class Animal {
public:
    virtual void speak() = 0;
    virtual ~Animal() {}; // Always a good thing if the class has virtual members
    string name;
};

class Dog : public Animal {
   ...  // unchanged
};

class AnimalQueue {
    list<Dog> dogs;
public:
    void enqueue(Animal* a) {
        Dog * d = dynamic_cast<Dog*>(a);
        dogs.push_back(*d); // This creates a copy of d and stores it
    }
    Dog dequeueDog() {
        Dog d = dogs.front(); // This creates a copy of the front element
        dogs.pop_front(); // Destroy the front element
        return d;
    }
};

int main() {

    AnimalQueue q;
    Dog * d;
    d = new Dog("Rex");
    q.enqueue(d);

    *d = q.dequeueDog();
    cout << d->name << endl;// Prints "Rex"
    d->speak(); // Prints WOFF

    delete d; // Free your memory

    return 0;
}

Example

Please notice that you also forgot to free your memory thus causing a memory leak. Either use a smart pointer or free your memory and be a good citizen.

---

Edit: OP edited his question to specify he also needs a dequeueAny method. I strongly recommend not to edit your question after it has been answered (requirements should be as static as possible). Anyway in that case I would suggest you to use pointers (or smart pointers) instead of copying objects around

class AnimalQueue {
    std::list<Animal*> animals;
public:
    void enqueue(Animal* a) {
        animals.push_back(a); // Copy the pointer
    }
    Animal* dequeue() {
        Animal *d = animals.front();
        animals.pop_front(); // Destroy the pointer
        return d;
    }
};

int main() {
    AnimalQueue q;
    std::unique_ptr<Dog> d = std::make_unique<Dog>("Rex");
    q.enqueue(d.get()); // This will now store the pointer to the object

    // Try with dog-specific command
    Animal *sameDog = q.dequeue(); // d.get() and sameDog are now pointing at the same object
    if (d.get() == sameDog)
        std::cout << "d.get() == sameDog" << std::endl;
    std::cout << sameDog->name << std::endl;// Prints "Rex"
    sameDog->speak();

    return 0;
}

Example

Answer 2

Dog* dequeueDog() {
    Dog * d = &(dogs.front());
    dogs.pop_front();
    return d;
}

There you are taking a pointer to the front item in the list, deleting that item (by calling pop_front) and then returning a dangling pointer.