11
votes

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;
}
4

4 Answers

12
votes

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;
}
4
votes

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.

2
votes

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)

0
votes

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).