Exampler / Prototype pattern help with cleanup

[thekashyap]

Hi,

I was implementing the Exampler / Prototype pattern just to brush up on C++.
Couldn't figure out how to do the cleanup at the end.
Problem is deleting all the prototypes from the map in base class at the end. I.e. deleting all stored pointers in Animal::examplesPtr , which is a map <string, Animal*> .

Code where the problem is (line 27-31 in Animal.cpp).
Full code for the whole project is attached.

Animal.h

#pragma once

#include "Common.h"
#include <map>

using std::map;
using std::pair;
using std::ostream;

class Animal {
    friend ostream& operator<< (ostream& o, Animal& b);

public:
    virtual ~Animal();
    static Animal* createAnimal (const string& type);
    virtual void speak() = 0;
    static void dump() ;

protected:
    Animal();
    Animal(const int id, const string& type);
    const int mIdInt;
    const string mTypeStr;

// To be implemented by derived classes..
protected:
    virtual string getType() = 0;
    virtual int getId() = 0;
    virtual Animal* createNewInstance() = 0;

private:
    typedef map<string, Animal*>::value_type t_typeToAnimalPair;
    typedef map<string, Animal*>::iterator t_examplesMapIterator;
    typedef map<string, Animal*> t_examplesMap;
    static t_examplesMap* examplesPtr;
    static void print(t_typeToAnimalPair it);
};

Animal.cpp

#include "Animal.h"
#include <algorithm>

ostream& operator<< (ostream& o, Animal& b) {
    return o << "[type=" << b.getType()
           << ", id=" << b.getId()
           << ", address=" << &b
           << ']';
}

// Not defined by size as the order of static var init
// isn't guaranteed, so the map might be used before it's init'd
Animal::t_examplesMap* Animal::examplesPtr ;

Animal::Animal (const int id, const string& type) : mIdInt(id), mTypeStr(type) {
    cout << "Inside Animal::Animal()" << endl;
    if (NULL == examplesPtr)
        examplesPtr = new t_examplesMap();

    if ( examplesPtr->find( type) == examplesPtr->end())
        examplesPtr->insert (t_typeToAnimalPair(string (type), this));
}

Animal::~Animal() {
    cout << "Inside Animal::~Animal()" << endl;

    // following results in infinite loop at delete it->second()
    // calls Animal::~Animal().
    t_examplesMapIterator it = examplesPtr->begin();
    for (; it != examplesPtr->end(); it++)
        delete it->second;

    examplesPtr->clear();
}

Animal* Animal::createAnimal (const string& type) {
    t_examplesMapIterator it = examplesPtr->find (type);
    if (examplesPtr->end() != it) {
        return it->second->createNewInstance();
    } else {
        string err = "Type " + type + " is not supported";
        throw err;
    }
}

void Animal::print (t_typeToAnimalPair p) {
    cout << "\t\t" << p.first << " = " << *(p.second) << endl;
}

void Animal::dump() {
    cout << "Animal::dump() -- " << endl
    << "\texamples.size() = " << examplesPtr->size() << endl;
    std::for_each (examplesPtr->begin(), examplesPtr->end(), print);
}

[JasonHippy]

From looking at the code, I think the infinite loop is happening because:
1. You call delete on an Animal, causing the flow of execution to enter the Animal class destructor.
2. Where the Animal destructor iterates through the static std::map it attempts to delete the first animal object in the map. So the flow of execution goes into the destructor for the first Animal object in the map.
3. The destructor for the 1st Animal object in the map then attempts to delete the first animal object in the static map (oops, that's itself!).
Now the destructor for the 1st Animal object in the std::map will recursively call itself until the program eventually runs out of memory and crashes.

I haven't looked at the rest of your code yet, or ran it through a debugger or anything, but I think that's what's going on!

Surely the cleanup for a static singleton object like this should really be done when your application ends, not each time an object of that class is deleted!

Otherwise, if there is a need to update the static when an Animal or Animal derived object is deleted, perhaps you need to find and remove that particular Animal instances entry from the static std::map rather than trying to remove all of them.

Anyway, hope this is of some help!
Cheers for now,
Jas.

[thekashyap]

Thanks. The reason for the infinite loop is quite obvious (it's written in the comment inside the code).
Finally I figured out that the no one should delete the examples as they are create statically by size and thus they should be deleted similarly. So the correct way to do it is do nothing. :)
Here the updated code:

Animal::~Animal() {
    cout << "Inside Animal::~Animal()" << endl;
    // static member examplesPtr shouldn't be deleted
    // as it's static !! D'tor is object specific.
}

In turn if the pattern is used in a context where the lifetime of examples is not same as the lifetime of application then there must be someone (some other class) that cleans up (read triggers cleanup of) the examplesPtr.