Downcasting shared pointer to derived class with additional functionality - is this safe?

Question

Consider the following outline:

class Base { /* ... */ };

class Derived : public Base
{
public:
    void AdditionalFunctionality(int i){ /* ... */ }
};

typedef std::shared_ptr<Base> pBase;
typedef std::shared_ptr<Derived> pDerived;

int main(void)
{
    std::vector<pBase> v;
    v.push_back(pBase(new Derived()));

    pDerived p1(  std::dynamic_pointer_cast<Derived>(v[0])  ); /* Copy */
    pDerived p2 = std::dynamic_pointer_cast<Derived>(v[0]);    /* Assignment */

    p1->AdditionalFunctionality(1);
    p2->AdditionalFunctionality(2);

    /* A */

    return 0;
}

Here I'm extending the base class with a derived class that adds functionality (the AdditionalFunctionality method).

First question, is this OK? I've read a lot of questions that say this is not okay and you should declare the additional functionality in the base class (often suggested as making them pure virtual methods in the base class). However, I don't want to do this. I want to extend the functionality of the base class, not just implement it differently. Is there a better solution to accomplish this goal?

Okay, so in this code I am also using an STL container to store these pointers which allows me to store pointers to both objects of type Base as well as objects of type Derived without slicing the objects.

Second question, this makes sense, right? I am, in fact, avoiding slicing by using pointers to base class objects rather than the base class objects themselves?

If I "know" that a certain pointer is to a Derived object, I then use std::dynamic_pointer_cast to cast the smart pointer.

Third question, this compiles without warning and works, but is it safe? Valid? Will it break the reference counting aspect of shared pointers and fail to delete my objects or delete them before I expect?

Lastly, I can do this cast using either the copy constructor or via assignment as shown for p1 and p2. Is there a preferred / correct way of doing this?

Similar questions:

• Downcasting shared_ptr<Base> to shared_ptr<Derived>? : This is very close, however the dervied class does not add additional functionality like mine does, so I'm not sure it's completely the same. Also, it uses boost::shared_ptr where I'm using std::shared_ptr (although I understand boost donated shared_ptr to the std library, so they're likely the same).

Thank you for your help.

---

Edit:

One reason I ask is that I realize that the following could be done (incorrectly):

    /* Intentional Error */
    v.push_back(pBase(new Base()));
    pDerived p3( std::dynamic_pointer_cast<Derived>(v[1]) );
    p3->AdditionalFunctionality(3); /* Note 1 */

Where I attempt to downcast a pointer to a Base object to a pointer of a Derived object and then call a method that is only implemented in the Derived class. In other words, the object pointed to doesn't define (or isn't even "aware of" the method).

This is not caught by the compiler, but may cause a segfault depending on how AdditionalFunctionality is defined.

Answer 1

Does the Base has a virtual destructor? If yes then it is safe to use downcasting. In your incorrect sample pDerived should be NULL in result, so you need to check the result of dynamic_pointer_cast every time.

Answer 2

If the container should never have base objects in it (I can't tell from the question but that's implied by your edit) then you should make the container hold derived objects instead, and then you have automatic access to the additional function.

If the container can have both types of objects, then it seems that you want to be able to treat all the objects as the base class within that container. In this case you almost certainly want to use polymorphism to do the right thing: Have a virtual interface that basically says "Do this work" and the parent version may do nothing at all. Then the child version of the method implements the additional functionality you need.

I think you may have a code smell that your objects are less related than you think. Are you inheriting to reuse, or to allow substitution? You may also want to reconsider what your public interface looks like.

All that said, should you decide to continue with your current design (which I would at least strongly review) I think your downcasting should be safe as long as you check the result of the dynamic cast for non-null before using it.

Answer 3

OK, first off, if you do this, you'll want to make sure that Base has a virtual destructor. Otherwise, you'll get undefined behavior when the vector goes out of scope. (The vector's destructor will call Base's destructor for each of its elements. If any elements are really a Derived -- KABOOM!) Other that that, what you've written is perfectly safe and valid.

But what's the point? If you have a container of objects, you want to be able to treat them all the same. (Loop over all of them and invoke a function on each, or whatever.) If you didn't want to treat them the same, why put them in one container? So you've got a vector that can hold pointers to Base or pointers to Derived -- how do you know which elements are of which type? Are you planning to just call dynamic_cast on each element every time you want to call AdditionalFunctionality to check to make sure that the element really points to a Derived? That's neither efficient nor idiomatic, and it basically defeats the whole point of using inheritance. You might as well just be using a tagged union.

You're using the wrong tool for the job. When people told you not to do this, it wasn't because it was unsafe or invalid, it's because you'll just end up making your code more complex than it needs to be.