views:

3839

answers:

9

When returning objects from a class, when is the right time to release the memory?

Example,

class AnimalLister 
{
  public:
  Animal* getNewAnimal() 
  {
    Animal* animal1 = new Animal();
    return animal1;
  }
}

If i create an instance of Animal Lister and get Animal reference from it, then where am i supposed to delete it?

int main() {
  AnimalLister al;
  Animal *a1, *a2;
  a1 = al.getNewAnimal();
  a2 = al.getNewAnimal();
}

The problem here is AnimalLister doesnot have a way to track the list of Animals Created, so how do i change the logic of such code to have a way to delete the objects created.

+23  A: 

I advise returning a std::tr1::shared_ptr (or boost::shared_ptr, if your C++ implementation does not have TR1) instead of a raw pointer. So, instead of using Animal*, use std::tr1::shared_ptr<Animal> instead.

Shared pointers handle reference tracking for you, and delete the object automatically if there are no references left to it.

Chris Jester-Young
The C++0x standard will have a unique_ptr<>, which does not have the overhead of a shared_ptr and still does what you need.
TonJ
std::auto_ptr<> is enough for this situation.
Martin York
@Martin: I'd advise against auto_ptr, as there are two revisions with fundamentally different behavior, they don't work well with STL containers etc. They are ok in this situation, but I find std::tr1 / boost ptrs much less ambigous.
peterchen
+8  A: 

The simpliest way is to return smart pointer instead of regular pointers. For example:

std::auto_ptr< Animal> getNewAnimal() 
{
  std::auto_ptr< Animal > animal1( new Animal() );
  return animal1;
}

If you are able to use TR1 or Boost, you can also use shared_ptr<>.

Igor Semenov
How do i implement the tracking ability?
bibstha
auto_ptr is in standard c++? this is pretty much easy it seems, so auto_ptr handles the garbage collection?
bibstha
auto_ptr is counter-intuitive, so don't use it unless you know exactly what it does. shared_ptr is recommended for normal use.
Chris Jester-Young
auto_ptr is not reference counting -- it's very difficult to get right if you start copy constructing it -- just use for local scope deleting -- never for returning or passing arguments.
Lou Franco
I agree with Chris, shared_ptr is more intuitive. But shared_ptr is not in Standard yet.Lou, why do you think, that auto_ptr should not be used for returning and passing arguments? Please, argue.
Igor Semenov
@Chris Jester-Young: Disagree. auto_ptr<> provides the correct symantics for this situation. It is a transfer of ownership. If you do not explicitly accept the transfer the object will be deleted (as expected). Shared Pointer has a completely different symantic meaning.
Martin York
I agree with Igor. auto_ptr can indeed be used for returning arguments (or passing arguments, if you are in fact relinquishing ownership of the object being passed in), and you can then transfer ownership to a shared_ptr should you need to.
Chris Jester-Young
it's largely broken. it's not copying, but moving (transfer-of-resources. it's the pointed to object ownership here) behind the scene. one should really avoid using it. its only safe use (which i know) is in raii with exception handling as a const auto_ptr, in place of a scoped_ptr
Johannes Schaub - litb
+7  A: 

Kind of a classic issue with pointers and allocated memory. It's about responsibility - who is responsible for cleaning up the memory allocated by the AnimalLister object.

You could store off a pointer to each of those allocated Animals in the AnimalLister itself and have it clean things up.

But, you do have a couple of pointers to Animals sitting there in main() that would be referencing memory that was deleted.

One of the reasons I think the reference counting solutions work better than rolling your own solution.

itsmatt
I fully agree the idea of the responsibility! The luxury of smart pointers makes us forget to think about it.
xtofl
Ahhh... with great powers, comes great responsibilities.
nullDev
+2  A: 

Or you could follow the COM-ish approach, and apply simple reference counting.

  • When you create the object, give it a reference value of 1 instantly
  • When anyone gets a copy of the pointer, they AddRef()
  • When anyone gives up their copy of the pointer, they Release()

If the reference count hits 0, the object deletes itself.

Its ultimately what the shared_ptr does under the hood, but it gives you more control over whats going on, and in my experience easier to debug. (Its also very cross-platform).

I haven't given shared_ ptr too much of a chance in my development as yet, so that may serve your purposes perfectly.

brianb
I think you meant shared_ptr, not auto_ptr. auto_ptr does not do reference counting, it does ownership-transfer semantics, which is not what most people want. :-)
Chris Jester-Young
Ooops ;-). As I said, not really used it yet (proven with great embarasment!)
brianb
+4  A: 
  1. shared_ptr (which works well),
  2. return a simple pointer and tell the user of your class that it is their animal now, and they have the responsibility to delete it when finished,
  3. implement a 'freeAnimal(Animal*)' method that makes it obvious that deletion of the animal pointer is required.

  4. An alternative way is to simply return the animal object directly, no pointers, no calls to new. The copy constructor will ensure the caller gets their own animal object that they can store on the heap or stack, or copy into a container as they desire.

So:

class AnimalLister 
{
Animal getAnimal() { Animal a; return a; }; // uses fast Return Value Optimisation
};

Animal myownanimal = AnimalLister.getAnimal(); // copy ctors into your Animal object

RVO means that returning the object instead of the pointer is actually faster (as the compiler doesn't create a new object and copies it into the caller's object, but uses the caller's object directly).

gbjbaanb
The problem with return-by-value is that you cannot return a subclass - it will be sliced to the Animal class
xtofl
But the second an third proposed solution are very useful: make it clear whose responsibility it is to free the animals.
xtofl
+2  A: 

The time to release the memory occupied by an object is when you don't need that particular object any more. In your particular case, the user of a class AnimalLister requested a pointer to a new allocated object of class Animal. So, he's the one that is responsible for freeing memory when he does need that pointer/object any more.

AnimalLister lister;
Animal* a = lister.getNewAnimal();
a->sayMeow();
delete a;

In my opinion, there's no need to over-engineer anything in this case. AnimalLister is just a factory that creates new Animal objects and that's it.

martinsb
+4  A: 

Depending on your usage, there are a couple of options you could go with here:

  1. Make a copy every time you create an animal:

    class AnimalLister 
    {
    public:
      Animal getNewAnimal() 
      {
        return Animal();
      }
    };
    
    
    int main() {
      AnimalLister al;
      Animal a1 = al.getNewAnimal();
      Animal a2 = al.getNewAnimal();
    }
    

    Pros:

    • Easy to understand.
    • Requires no extra libraries or supporting code.

    Cons:

    • It requires Animal to have a well-behaved copy-constructor.
    • It can involve a lot of copying if Animal is larg and complex, although return value optimization can alleviate that in many situations.
    • Doesn't work if you plan on returning sub-classes derived from Animal as they will be sliced down to a plain Animal, losing all the extra data in the sub-class.
  2. Return a shared_ptr<Animal>:

    class AnimalLister 
    {
    public:
      shared_ptr<Animal> getNewAnimal() 
      {
        return new Animal();
      }
    };
    
    
    int main() {
      AnimalLister al;
      shared_ptr<Animal> a1 = al.getNewAnimal();
      shared_ptr<Animal> a2 = al.getNewAnimal();
    }
    

    Pros:

    • Works with object-hierarchies (no object slicing).
    • No issues with having to copy large objects.
    • No need for Animal to define a copy constructor.

    Cons:

    • Requires either Boost or TR1 libraries, or another smart-pointer implementation.
  3. Track all Animal allocations in AnimalLister

    class AnimalLister 
    {
      vector<Animal *> Animals;
    
    
    public:
      Animal *getNewAnimal() 
      {
        Animals.push_back(NULL);
        Animals.back() = new Animal();
        return Animals.back();
      }
    
    
      ~AnimalLister()
      {
         for(vector<Animal *>::iterator iAnimal = Animals.begin(); iAnimal != Animals.end(); ++iAnimal)
            delete *iAnimal;
      }
    };
    
    
    int main() {
      AnimalLister al;
      Animal *a1 = al.getNewAnimal();
      Animal *a2 = al.getNewAnimal();
    } // All the animals get deleted when al goes out of scope.
    

    Pros:

    • Ideal for situations where you need a bunch of Animals for a limited amount of time, and plan to release them all at once.
    • Easily adaptable to custom memory-pools and releasing all the Animals in a single delete.
    • Works with object-hierarchies (no object slicing).
    • No issues with having to copy large objects.
    • No need for Animal to define a copy constructor.
    • No need for external libraries.

    Cons:

    • The implementation as written above is not thread-safe
    • Requires extra support code
    • Less clear than the previous two schemes
    • It's non-obvious that when the AnimalLister goes out of scope, it's going to take the Animals with it. You can't hang on to the Animals any longer than you hang on the AnimalLister.
Eclipse
A: 

I really like Josh's answer, but I thought I might throw in another pattern because it hasn't been listed yet. The idea is just force the client code to deal with keeping track of the animals.

class Animal
{
...
private:
  //only let the lister create or delete animals.
  Animal() { ... }
  ~Animal() { ... } 
friend class AnimalLister;
...
}

class AnimalLister 
{
  static s_count = 0;

public:
  ~AnimalLister() { ASSERT(s_count == 0); } //warn if all animals didn't get cleaned up

  Animal* NewAnimal() 
  {
    ++count;
    return new Animal();
  }

  void FreeAnimal(Animal* a)
  {
    delete a;
    --s_count;
  }
}
BigSandwich
A: 

In a thorough discussion by Scott Meyers, he concludes that using shared_ptr or auto_ptr is the best.

Amit Kumar