C++ : about memory management

Question

I'm a bit new to C++ and been doing programming in Obj-C and Java up until now.

Say, I have a class:

class Person {

private:
   Wife *current_wife;
   //.....
};

So obv I need to implement a setter method to change the Wife instance variable.

Like this:

Person::SetCurrentWife (Wife *new_wife) {

    current_wife = new_wife;
}

That would be a shalllow copy.

So somewhere from the main loop or something I call:

Person *some_person = new Person();
...
Wife *wife = new Wife ();
some_person->SetCurrentWife(wife);

So I'm confused: will there be a memory leak here? Should I delete the wife object here or in Person's destructor? In Obj-C I would release the current wife and then send a retain message to wife object above, but what is the proper way of doing setter methods in C++?

Answer 1

It depends on what you're trying to do. First, as Kerrek SB has commented, you don't want to use pointers if value semantics can be applied: if Wife is copyable and assignable, there's almost no reason to allocate it dynamically. In this case, however, I'm guessing that Wife derives from Person (although perhaps a decorator for Person would be more appropriate, since the fact that a given Person isA Wife can change over time), that there might even be types derived from Wife (and that Person::current_wife might want to hold one of these), and that in fact, Wife has identity; you don't want copies of the same wife all over the place.

If this is the case, then you really have to define a protocol for the interactions of other classes with Wife. Typically, the lifetime of Wife will not depend on the Person who holds it (although if it is a decorator, it might), so Person should just hold a pointer to it, as you've done. Most likely, the Wife object will have various functions which—implicitly or explicitly—control its lifetime: you might have something like:

void Wife::die()
{
    //  ...
    delete this;
}

for example. In that case, however, whoever is married to Wife will have to be informed, so that current_wife doesn't point to a dead spouse. Typically some variant of the observer pattern can be used for this. (Note that you have exactly the same issue in Java; you don't want Person::current_wife to point to a dead Wife. So you'd still need a Wife::die() function, and the observer pattern to notify the spouse.)

In cases like this (which in my experience represent the vast majority of dynamically allocated objects in C++), about the only difference between C++ and Java is that C++ has a special syntax for calling the “destructor”; in Java, you use the usual function call syntax, and you can give the function any name you wish (although dispose seems widely used). The special syntax allows the compiler to generate additional code to release the memory, but all of the other activities associated with the end of the object lifetime still have to be programmed (in the destructor, in C++—although in this case, it might make sense to put some of them directly in the Wife::die function).

Answer 2

You should use a smart pointer.

If you use regular pointer, practice caution!

You should delete the old current_wife member both on the destructor and on the set method. Setting a new wife will lead to the memory leak of the old one, as the pointer to that allocated memory is lost (unless you manage the memory outside the class - see below).

But even doing so, you need to make sure that no-one outside the class can delete the member. You have to decide whether memory management is left to the class or dispached to the outside of the class, and stick to it.

Answer 3

Smart pointers can help you

using boost::shared_ptr; // or std::shared_ptr, or std::tr1::shared_ptr or something like this
class Person { 

private: 
   shared_ptr<Wife> current_wife; 
   //..... 
}; 

Person::SetCurrentWife (shared_ptr<Wife> new_wife) { 

    current_wife = new_wife; 
} 

And now you should not delete any wife at all.

shared_ptr<Person> some_person ( new Person );
...  
shared_ptr<Wife> wife ( new Wife );  
some_person->SetCurrentWife(wife);