Acceptable use of smart pointers?

[JayhawkZombie]

First, I'd like to thank you for your help when I've asked questions, and for being patient with me.

You recommended not using manual memory management, and I agree - it was getting complicated and tedious, and I was worried about memory leaks.

I've decided to refactor to use smart pointers (and clean it up a little), but I want to be sure I actually understand how to use these guys and incorporate them into my multiple levels of inheritance.

So here goes nothing. A base class from which I derive, 2 derived classes, and a container class.
I won't post full source. It wouldn't be productive and would just deter those from helping. But, if you want to see it, I put it on GitHub: https://github.com/JayhawkZombie/Random/tree/master/Smart-Pointers

Main:

#include <iostream>
#include <vector>
#include <memory> //Smart pointers

#include "../Base.h"
#include "../Derived1.h"
#include "../Derived2.h"
#include "../Container.h"

int main()
{
        //Container has a vector of smart_ptr's to the Base class
	std::shared_ptr<Container> container = std::make_shared<Container>();

	std::shared_ptr<Derived1> derPtr1 = std::make_shared<Derived1>();
	std::shared_ptr<Derived2> derPtr2 = std::make_shared<Derived2>();

	std::cout << "Sending new object: " << derPtr1 << std::endl;
	container.get()->addNewObejct(derPtr1);

	std::cout << "Sending new object: " << derPtr2 << std::endl;
	container.get()->addNewObejct(derPtr2);

	container.get()->makeCallbacks();
	container.get()->callOverridenVirtual();
	container.get()->deleteAllObjects();

	return 0;
}

Sending new object: 00899C38
Adding new object: 00899C38
Sending new object: 00899E58
Adding new object: 00899E58
Making callbacks
Custom callback for Derived1: 0
0, 1, 4, 9, 16, 25, 36, 49, 64, 81,
Custom callback for Derived2: 1
mustOverride in Derived1
mustOverride in Derived2
Resetting object: 0
Resetting object: 1
~Derived2
        ~Base
~Derived1
        ~Base
Press any key to continue . . .

[helios]

std::shared_ptr overloads operator->(). You don't need to call get() just to access members of the pointed object.

[cire]

shared_ptr is usually overkill. Do you actually need to share ownership?

void Container::deleteAllObjects()
{
	for (auto i = 0; i < m_numObjects; i++)
	{
		std::cout << "Resetting object: " << i << std::endl;
		m_objects[i].reset();
	}
}

Would probably be better written as:

void Container::deleteAllObjects()
{
    m_objects.clear();
}

*cough* Also, if you're using a container that keeps track of its size, you might want to take advantage of that. *cough*

[JayhawkZombie]

I suppose I do not have to, no. I could (and probably should) use unique_ptr instead. I will make adjustments.

*facepalm* Thanks. Threw size and capacity together for some reason. I've been doing C all semester and got used to manually keeping track of the number of elements. It got kind of drilled in. Now I have to adjust back.

[JayhawkZombie]

OK, so, can I make sure I have the difference between them correct?
I should really only use shared_ptr if they need to share responsibility?
There's no need to share ownership, so I can 'move' it to the container since it will be the container who decides when it gets deleted?

[helios]

std::unique_ptr can't be used in an STL container, though.

[cire]

OK, so, can I make sure I have the difference between them correct?

More or less. ;)

I think the answers at:
http://stackoverflow.com/questions/7657718/when-to-use-shared-ptr-and-when-to-use-raw-pointers
give some good reasoning/guidelines.

If you don't recognize some of the smart pointers mentioned, you'll find them in Boost.

[cire]

helios wrote:

std::unique_ptr can't be used in an STL container, though.

I think you're thinking of the now defunct auto_ptr.

[JayhawkZombie]

Well, if I can't use unique_ptr in an STL container, then I'll have to find another solution, or use shared pointers. I need to be able to keep the use of the STL container.

Or could I not 'move' the unique_ptr into the vector as I call push_back? Once I create it in main and give it to the container, the container will then mange the lifetime of the object.

vec.push_back(std::move(my_unique_ptr)); ?

Clearly, I do not understand this. I will have to return to my books.

[JLBorges]

> Could I not 'move' the unique_ptr into the vector as I call push_back?
> Once I create it in main and give it to the container, the container will then mange the lifetime of the object.
> vec.push_back(std::move(my_unique_ptr)); ?

Yes. Yes.

Or even better, move an rvalue: vec.push_back( std::make_unique<my_class>( /* constructor args */ ) );

[helios]

Odd. I don't know why I remembered it was not possible. Never mind, then.

[JayhawkZombie]

I do like the rvalue move, but the object the unique pointer is pointing to would be created, modified after creation, and then moved into the vector after some manipulation.
Hmm, I'll consider manipulating the constructor so that doesn't need to be done.

What would be the benefit of moving an rvalue versus creating it, doing the manipulation manually, and then copying it into the vector?

It sounds like unique_ptr is what I want, since only the container will be managing the lifetime of the object.

[JLBorges]

> I'll consider manipulating the constructor so that doesn't need to be done.

Or, if this special initialisation is not part of the abstract interface of the class, write a helper function that returns a std::unique_ptr<> as a prvalue.


> What would be the benefit of moving an rvalue versus creating it,
> doing the manipulation manually, and then copying it into the vector?

The code becomes cleaner. Particularly in the presence of exceptions (no manual clean up is required).

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

struct A
{
    A( int v ) : value(v)
    {
        if( v > 100 ) throw std::domain_error("error while constructing object") ;
        std::cout << "A::constructor\n" ;
    }
    virtual ~A() { std::cout << "A::destructor\n" ; }

    int value ;
};

struct B : A
{
    B( int x, int y ) : A(x), another_a(y) { { std::cout << "B::constructor\n" ; } }
    ~B() { std::cout << "B::destructor\n" ; }

    A another_a ;
};

std::unique_ptr<B> make_B( std::istream& stm )
{
    int x ;
    int y ;
    if( stm >> x >> y ) { std::clog << x << ' ' << y << '\n' ; return std::make_unique<B>( x, y ) ; }
    else throw std::runtime_error( "input error" ) ;
}

int main()
{
    std::vector< std::unique_ptr<A> > seq ;

    try
    {
        seq.push_back( make_B(std::cin) ) ; // 25, 30 : ok
        std::cout << "ok. first object was pushed back\n\n" ;

        seq.push_back( make_B(std::cin) ) ; // 25, 330 : throws after base class is constructed
        std::cout << "second object was pushed back\n\n" ;
    }
    catch( const std::exception& e ) { std::cerr << "*** error **** " << e.what() << "\n\n" ; ; }

    try
    {
        seq.push_back( make_B(std::cin) ) ; // ab, cd : input error
        std::cout << "third object was pushed back\n\n" ;
    }
    catch( const std::exception& e ) { std::cerr << "*** error **** " << e.what() << "\n\n" ; ; }

    std::cout << "\n* end of program *\n" ;
}

25 30
A::constructor
A::constructor
B::constructor
ok. first object was pushed back

25 330
A::constructor
A::destructor
*** error **** error while constructing object

*** error **** input error


* end of program *
B::destructor
A::destructor
A::destructor

http://coliru.stacked-crooked.com/a/7d8feaf0d9db1348

[JayhawkZombie]

Who keeps reporting JLBorges? Ridiculous. All he does it help. Thank you for your help, everyone. I'm going to take some time to refactor. I've build a mini version of my project with unique pointers and I'll build it back up from there. 90% of the code can just be dragged and dropped anyway.

[cire]

Who keeps reporting JLBorges?

I think he just likes his posts in red.

[JLBorges]

> I think he just likes his posts in red.

Yes!

I'm vain enough to be pleased that my posts stand out from the crowd, and naive enough to believe that (more) people would read my posts with (more) care.

It has a benign side-effect too: I've become a little more meticulous about the content of my posts.

Let it be; it does no harm to anyone; even newbies are able to see through this.
(When this troll-reporting started, I got carried away and actually self-reported a few posts of my own.)

[helios]

What did you put in the reason field, just out of curiosity?

[JLBorges]

> What did you put in the reason field, just out of curiosity?

"Might as well highlight this post too" (or something to that effect).