11

I want to store objects of classes derived from a common interface (abstract class) in a std::vector of that abstract class. This vector should be filled in a loop and usually I would call the constructor of a class and push the created object into the vector.

As I understand, in case of an abstract class I can only store pointers to that class, so I need to push_back pointers of the derived classes. However, I am not sure about the scope of these newly created objects.

Please, have a look at the code below. This code compiles and works fine but my questions are:

a) Are the objects guaranteed to exist in the second for-loop in the main function? Or might they cease existing beyond the scope of the loop in which they are created?

b) Are all objects' destructors called or might there be memory leaks?

#include<vector>
#include<iostream>
class Interface {
    public:
    Interface( int y ) : x(y) {}
    virtual ~Interface() {}
    virtual void f() = 0;
    int x;  
};

class Derived_A : public Interface {
    public:
    Derived_A( int y ) : Interface(y) {}
    void f(){ return; }
};

class Derived_B : public Interface {
    public:
    Derived_B( int y ) : Interface(y) {}
    void f(){ return; }
};


int main()
{
    std::vector<Interface*> abstractObjects;
    int N = 5;
    for(int ii = 0; ii < N; ii++ )
    {
        abstractObjects.push_back( new Derived_A(ii) );
        abstractObjects.push_back( new Derived_B(ii) );
    }

    for(int ii = 0; ii < abstractObjects.size(); ii++ )
    {
        abstractObjects[ii]->f();
        std::cout << abstractObjects[ii]->x << '\t' << std::endl;
    }


    for(int ii = 0; ii < abstractObjects.size(); ii++ )
    {
        delete abstractObjects[ii];
    }

    return 0;
}
seyfe
  • 456
  • 5
  • 23

4 Answers4

12

This is a perfect case for smart pointers. You can store the pointers in a unique_ptr which is a RAII type. When the unique_ptr goes out of scope it will autmaticlly delete the memory for you.

    //...
    std::vector<std::unique_ptr<Interface>> abstractObjects;
    int N = 5;
    for(int ii = 0; ii < N; ii++ )
    {
        abstractObjects.push_back( std::make_unique<Derived_A>(ii) );
        abstractObjects.push_back( std::make_unique<Derived_B>(ii) );
    }

    for(auto & e : abstractObjects)  // ranged based for loop
    {
        e->f();
        std::cout << e->x << '\t' << std::endl;
    }
    // no need to do anything here.  the vector will get rid of each unique_ptr and each unique_ptr will delete each pointer
    return 0;
}
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • If using c++14 anyway why not for range loop then? – Slava Jul 14 '15 at 15:34
  • @Slava Thanks for the suggestion. I edited the code. – NathanOliver Jul 14 '15 at 15:39
  • Thanks! What would happen if I had a function `f2()` that creates the vector as you suggested and afterwards creates an object `Obj(std::vector >)` using this exact vector and returns this `Obj` from the function? Would the vector (and its elements) still exist in the function calling `f2()`? – seyfe Jul 14 '15 at 15:48
  • @seyfe `unique_ptr is non copyable. Because of that you can only take references to the vector containing them or you can [move](http://stackoverflow.com/questions/3106110/what-are-move-semantics) the vector to where you want it – NathanOliver Jul 14 '15 at 15:53
  • Nice reference, I enjoyed reading it! If I get you right, this would mean calling the constructor `Obj( std::vector >)` as `std::vector > v; Obj o( std::move( v ) );` moves the ownership of vector `v` to the object `o` and therefore the vector will be destroyed at the same time `o` is destroyed? – seyfe Jul 14 '15 at 16:10
  • @seyfe I think this would be better suited as its own question. I would give it a shot and if you get a problem with it then post a question. – NathanOliver Jul 14 '15 at 16:13
  • The behavior of passing a `std::vector >` is described here: http://stackoverflow.com/questions/8114276/how-do-i-pass-a-unique-ptr-argument-to-a-constructor-or-a-function – seyfe Jul 15 '15 at 07:42
4

Let me address your points.

a) Are the objects guaranteed to exist in the second for-loop in the main function? Or might they cease existing beyond the scope of the loop in which they are created?

When you call the keyword new, the objects will exist until you explictly call delete on them to free the associated memory. If you had instead created the objects on the stack, they would fall out of scope after the first loop terminated.

b) Are all objects' destructors called or might there be memory leaks?

Yes, you are correctly calling the destructors of each object in your final loop, and there generally will not be memory leaks. However, if an exception is thrown before you reach the final loop, the allocated memory will not be reclaimed and you will have a leak. See this post.

You can, however, improve your code by using smart pointers, which solves that problem by automatically reclaiming memory. Use std::make_unique<Derived_A>(ii) instead of new Derived_A(ii), and when the vector goes out of scope, it will automatically free the associated memory for each object it contains, removing the need to explicitly call the destructors yourself in the final loop.

Community
  • 1
  • 1
Aaron Zou
  • 414
  • 3
  • 9
  • 1
    "and there should not be memory leaks." probably you should mention that exception may also lead to a memory leak and that's another reason to use smart pointers. – Slava Jul 14 '15 at 15:40
2

Yes the objects still exist in the outside the scope of your first for loop so you were correct to delete them.

No not all objects' destructors are automatically called. If you new something, then you must delete it. You could (and should) however use smart pointers.

std::vector<std::unique_ptr<Interface>> abstractObjects;
int N = 5;
for(int ii = 0; ii < N; ii++ )
{
    abstractObjects.push_back( std::make_unique<Derived_A>(ii) );
    abstractObjects.push_back( std::make_unique<Derived_B>(ii) );
}

Now you do not have to delete anything, the destructors will be called when the vector falls out of scope (and therefore so do all of it's elements)

Cory Kramer
  • 114,268
  • 16
  • 167
  • 218
0

I suggest using a vector of smart pointers (instead of raw owning pointers). While raw observing pointers are fine, having a vector of raw owning pointers can be a potential source of leaks.

For a non-shared ownership, consider using vector<unique_ptr<Interface>>, and use std::make_unique to dynamically create new items to be added to the vector.

Using a vector of smart pointers allows you writing simpler and clearer code, since the cleanup is automatically done thanks to C++ destructors (in other words, all the manual cleanup code required with a vector of raw owning pointers just disappears).

Mr.C64
  • 41,637
  • 14
  • 86
  • 162