Getting polymorphism to work in a C++ map without memory leaks?

Question

It's been a long³ time since I programmed in C++. My polymorphism isn't working: the map<string, Base> converts my ArmyBase and NavyBase objects to Base objects when I add them to the map, so GetDescription() returns an empty string rather than the values I set via ArmyBase::SetDescription() and NavyBase::SetDescription(). Here's the extremely rough pseudo-code:

class Base
{ protected:
    string STR__Description;  // Pardon my non-standard style
  public:
    virtual
    string GetDescription()
    { return STR__Description;
    }
    void   SetDescription( string str__Description )
    { STR__Description = str__Description;
    }
}

class ArmyBase: public Base
{ public:
    string GetDescription()
    { return STR__Description + " (Army)";
    }
}

class NavyBase: public Base
{ public:
    string GetDescription()
    { return STR__Description + " (Navy)";
    }
}

It sounds like map<string, Base*> causes memory leaks and I'd rather not upgrade mid-project to use shared_ptr. Would storing the derived-class instances in a container that "destructs" them properly allow me to use the pointer map for polymorphism without risk of memory leakage?

Base                         base;
ArmyBase                     base_Army;
set<ArmyBase>                set__ArmyBases;
map<string, Base*>::iterator iter_Bases;
map<string, Base*>           map__Bases;
NavyBase                     base_Navy;
set<NavyBase>                set__NavyBases;
...
while( ... )
{   base_Army = ArmyBase();
    base_Navy = NavyBase();
    ...
    set__ArmyBases.insert(                                        base_Army   );
    map__Bases.insert(     pair<string, Base*>( "Boca Raton",    &base_Army ) );
    ...
    set__NavyBases.insert(                                        base_Navy   );
    map__Bases.insert(     pair<string> Base*>( "NAS Pensacola", &base_Navy ) );
    ...
    base = iter_Bases->second;
    std::cout << ..." " << base->GetDescription() << std::endl;
}

Desired output from map__Bases:

Boca Raton ... (Army)
NAS Pensacola ... (Navy)
...

Answer 1

You should create and use pointers of objects using "new" to use polymorphism in c++. What you are experiencing is called object slicing.

Answer 2

The problem is that the map entries point to the adresses of the objects which you created on the stack. You copy those objects into your sets, but you don't store the addresses of the copies in your maps. When the original objects fall out of scope, they get deleted automatically and thus your map entries become invalid.

I think the best way to solve your problem is to allocate your objects on the heap and store pointers in your containers. This, of course, requires you to be careful about memory management. I know three options to handle this:

1. Manual memory management: Delete objects when you erase them from your container. This is dangerous and error-prone, of course, but with some care, you can make it work. I usually do this by wrapping the container in another class that will manage the objects, i.e., the wrapper has methods such as add(Base* base) and remove(Base* base), and it will also delete all objects in the container in its destructor. Of course, you must still take care not to delete such managed objects outside of the wrapper.

2. Smart pointers: Use either shared_ptr or unique_ptr (depending on ownership semantics) and store those in the container. The smart pointers will take care of deleting the objects when they are removed from the container.

3. Use a custom allocator. You can parametrize the std containers with allocators which should allow the container to delete the objects when they are removed from the map. However, I have never done this and can't comment on whether it's a good idea. I hear that writing custom allocators is quite difficult to get right.

I would suggest either 1 and 2. I think it depends on taste and on other requirements which one you actually use, but if you use the smart pointer option, then make sure that you choose the smart pointer that models your particular ownership semantics (most likely unique_pointer).

Answer 3

To little information to really work out exactly what your problem is. But I'll have a stab going from what's available.

How do you detect memory leakage? One obvious problem if ArmyBase/NavyBase contains non-POD members is that its destructor won't be called if you delete from base, hence you need a virtual destructor for Base.

The second solution you've suggested using several different containers is almost certainly the wrong approach in this case. A different container for each derived type defeats the whole purpose of polymorphism in the first place. Besides, your loop doesn't make any sense at all. set__ArmyBases/set__NavyBases contains identical default constructed objects, while map__Bases just contains pointers to the 2 global variables base_Navy & base_Army that gets re initialized every frame.

Answer 4

Apart from a ridiculous amount of typos in your code (which is why you should always provide a SSCCE), your code can be made to work after two small corrections:

• write a small comparison function object Cmp that compares the descriptions by following the pointers to Base
• change your set<ArmyBase> and set<NavyBase> as set<Base*, Cmp> and set<Base*, Cmp>.

Code:

#include <string>
#include <set>
#include <map>
#include <iostream>

using namespace std;

class Base
{ protected:
    string STR__Description;  // Pardon my non-standard style
  public:
    virtual
    string GetDescription() const
    { return STR__Description;
    }
    void   SetDescription( string str__Description )
    { STR__Description = str__Description;
    }
};

class ArmyBase: public Base
{ public:
    string GetDescription() const
    { return STR__Description + " (Army)";
    }
};

class NavyBase: public Base
{ public:
    string GetDescription() const
    { return STR__Description + " (Navy)";
    }
};

class Cmp
{
public:
    bool operator()(Base const* lhs, Base const* rhs) const
    { return lhs->GetDescription() < rhs->GetDescription(); };
};

int main()
{

    Base                         base;
    ArmyBase                     base_Army;
    set<Base*, Cmp>           set__ArmyBases;
    map<string, Base*>      map__Bases;
    map<string, Base*>::iterator         iter_Bases;
    NavyBase                     base_Navy;
    set<Base*, Cmp>          set__NavyBases;


    base_Army = ArmyBase(); 
    base_Navy = NavyBase();

    set__ArmyBases.insert(                   &base_Army   );
    map__Bases.insert(     pair<string, Base*>( "Boca Raton",    &base_Army ) );


    set__NavyBases.insert(                   &base_Navy   );
    map__Bases.insert(     pair<string, Base*>( "NAS Pensacola", &base_Navy ) );

    for (auto b: map__Bases)
        std::cout << b.first << "..." << b.second->GetDescription() << std::endl;
}

Live example with output.

There is no object slicing or memory errors going on. However, you are using raw pointers to scoped objects (i.e. their descructors are called when they go out of scope).

The recommended way for having containers of polymorphic objects is to use std::map<std::string, std:unique_ptr<Base>> and to allocate these through std::make_unique<NavyBase>("NAS Penscalola").

Answer 5

You should be able to get something close to what you want using Boost Intrusive associative containers. Unlike the Standard Library containers, they do not store a copy but a reference to your object. This should allow your polymorphism to work when retrieving the object from the intrusive container.

From the documentation:

The main difference between intrusive containers and non-intrusive containers is that in C++ non-intrusive containers store copies of values passed by the user. ...

On the other hand, an intrusive container does not store copies of passed objects, but it stores the objects themselves. ...

   acme_intrusive_list<MyClass> list;

   MyClass myclass;
   list.push_back(myclass);

   //"myclass" object is stored in the list
   assert(&myclass == &list.front());

In practical terms, this means you should use one or more simple Standard Library containers (like list<>) to actually store your derived instances, but you add those instances to the Boost.Intrusive container. Since the Boost.Intrusive container holds references, if you destroy the actual instances, the Boost.Intrusive container will be corrupted.