I am trying to implement factory pattern by registering the function pointers of the derived class to the factory in a static map(member of the factory)and creating objects by looking up the map. But I am getting a segmentation fault in doing this.
Code Snippet:
factory.cpp
typedef Shape* (*Funcptr)();
std::map<int,Funcptr> Factory::funcmap;
int Factory::registerCreator(int ShapeID, Shape *(*CFuncptr)()) {
Factory::funcmap[ShapeID] = CFuncptr;
return 1;
}
Shape* Factory::CreateObject(int ShapeID) {
std::map<int,Funcptr>::iterator iter;
iter = funcmap.find(ShapeID);
if(iter != funcmap.end()){
return iter->second();
}
return NULL;
}
factory.h
class Factory {
public:
Factory();
virtual ~Factory();
static int registerCreator(int, Shape *(*CFuncptr)());
Shape* CreateObject(int);
private:
static std::map<int,Funcptr> funcmap;
};
Square.cpp
static Shape *SquareCreator() {
return new Square;
}
static int SquareAutoRegHook = Factory::registerCreator(1,SquareCreator);
On creating the object for Factory in the main file a segmentation fault occurs. Can you please suggest if I am doing something wrong. I am using CppUTest for TDD and not sure how to debug this.
There is no guarantee about the order of creation of static objects that are defined in different translations uints* so you have a 50/50 shot as to which would happen first, the initializtion of Factory::funcmap
or Factory::registerCreator(1,SquareCreator)
and Undefined Behavior Russian Roulette is not a good game to play.
A common approach to deal with this, and one that described in Item 4 of the third edition of Scott Meyer's Effective C++ is to use local static objects instead of global static objects. In this case it means changing Factory
to look like this:
class Factory {
public:
Factory();
virtual ~Factory();
static int registerCreator(int, Shape *(*CFuncptr)());
Shape* CreateObject(int);
private:
static std::map<int,Funcptr> & GetFactoryMap() {
static std::map<int,Funcptr> funcmap;
return funcmap;
}
};
and changing Factory::registerCreator
to this:
int Factory::registerCreator(int ShapeID, Shape *(*CFuncptr)()) {
GetFactoryMap()[ShapeID] = CFuncptr;
return 1;
}
This way funcmap will be initialized the first time registerCreator is called and will never be used uninitialized.
*Or, roughly speaking, different .cpp files if you are not familar with the term translation unit
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With