CODEWITHSUNDEEP
c++smart-pointers
Vinzent Meier
Vinzent Meier

Reputation: 45

Replacing/refactoring of naked pointers

I want to replace traditional naked pointer usage in a class inheritance situation.

Example for what I mean:

#include <iostream>
#include <vector>
#include <memory>

using namespace std;

class Base
{
public:
  void doBaseStuff ()
  {
    cout << "base stuff\n";
  }
};

class Derived:public Base
{
public:
  void doDerivedStuff ()
  {
    cout << "derived stuff\n";
  }
};


int main ()
{
 // vector to hold pointers
  vector < Base * >baseCollection;

// generate pointers
  Derived *derivedPtr = new Derived;
  Base* basePtr = new Base;
  
 // use derived pointer for something
  derivedPtr->doDerivedStuff ();
  
 // fill vector with pointers
  baseCollection.push_back (basePtr);
  baseCollection.push_back (derivedPtr);

 // iterate some 'interface' method
 for (auto & element:baseCollection)
 {
   element->doBaseStuff ();
 }

  return 0;
}

Is refactoring this as shown below the correct way? I use C++17. std::unique_ptr instead of std::shared_ptr won't compile in this case (line: baseCollection.push_back(derivedPtr);).

#include <iostream>
#include <vector>
#include <memory>

using namespace std;

// sample classes
class Base
{
public:
  void doBaseStuff ()
  {
    cout << "base stuff\n";
  }
};

class Derived:public Base
{
public:
  void doDerivedStuff ()
  {
    cout << "derived stuff\n";
  }
};


int main ()
{
 // vector to hold pointers
 vector<shared_ptr<Base>> baseCollection;
 
 // generate pointers
 shared_ptr<Base> basePtr = make_shared<Base>();
 shared_ptr<Derived> derivedPtr = make_shared<Derived>();
 
 // use derived pointer for something
 derivedPtr->doDerivedStuff();
 
 // fill vector with pointers
 baseCollection.push_back(basePtr);
 baseCollection.push_back(derivedPtr); // implicit cast from shared_ptr<Derived> to shared_ptr<Base> !?
 
 // iterate some 'interface' method
 for(auto& element : baseCollection)
 {
     element->doBaseStuff();
 }

  return 0;
}

The output is the following in both cases, as expected:

derived stuff
base stuff
base stuff

Upvotes: 1

Views: 332

Answers (2)

prog-fh
prog-fh

Reputation: 16825

You can use std::unique_ptr instead of std::shared_ptr (it is recommended indeed, because ownership is clearly handled in your application, and not left unspecified to a kind-of garbage-collector), but you have to move them into the vector in order to transfer (unique) ownership.

 vector<unique_ptr<Base>> baseCollection;
 
 // generate pointers
 unique_ptr<Base> basePtr = make_unique<Base>();
 unique_ptr<Derived> derivedPtr = make_unique<Derived>();
 
 // use derived pointer for something
 derivedPtr->doDerivedStuff();
 
 // fill vector with pointers
 baseCollection.push_back(std::move(basePtr));
 baseCollection.push_back(std::move(derivedPtr));

Of course, you must not use your basePtr and derivedPtr variables afterwards because they don't own these objects anymore.

An alternative would be to use temporaries:

 baseCollection.push_back(make_unique<Base>());
 baseCollection.push_back(make_unique<Derived>());

Note that when dealing with unique_ptrs you can still use the raw pointers (with the .get() member-function) but the meaning is different: I know an object which is owned by someone else (a unique_ptr here).

And as stated in the comments, in order to ensure a correct destruction, the destructor of Base should be virtual ~Base()=default; because the destructor of baseCollection does not know that the Base pointer actually points to a Derived (probably not the same size, dynamic dispatch is needed). This is not related to unique_ptr nor shared_ptr; the problem existed with raw-pointers. And to go further, since we have polymorphic types we should disable (=delete) copy/move-constructors/assign in order to prevent slicing, but we are far from the original question.

Upvotes: 5

eerorika
eerorika

Reputation: 238331

Is refactoring this as shown below the correct way?

The behaviour of the refactored program is different from the first. The first program leaks memory, while the refactored one does not. That's of course typically a good thing as long as it is intentional.

I use C++17. std::unique_ptr instead of std::shared_ptr won't compile in this case.

That's because you make a copy of the pointer, and unique pointer aren't copyable. You can move the pointer instead. However, unique pointers to base have another problem that shared pointers don't have: If you delete a derived object through a pointer to base, and the destructor of base isn't virtual, then the behaviour of the program will be undefined. This can be easily fixed by making the destructor of the base virtual.

So, as long as you don't copy the pointer, and you make the destructor virtual, there would be no need to pay for the overhead of shared ownership.

Upvotes: 2

Related Questions