Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is the typical C++ implementation of Factory class flawed?

I have the need to implement factory class in C++, but when I was thinking about that, I found one big problem that I couldn't solve, and I found out, that all factory implementation examples around are flawed in the same way. I'm probably the one who is wrong, but please tell me why.

So here is simple "typical" factory implementation, it allows me to register new objects without changing the Factory class.

//fruit.h
class Fruit
{
protected :
  int count;
public :
  Fruit(int count) : count(count) {}
  virtual void show() = 0;
};

// factory.h
/** singleton factory */
class Factory
{
  typedef Fruit* (*FruitCreateFunction)(int);
  static Factory* factory;
  std::map<std::string, FruitCreateFunction> registeredFruits;
public :
  static Factory& instance()
  {
    if (factory == NULL)
      factory = new Factory();
    return *factory;
  }
  bool registerFruit(const std::string& name, Fruit* (createFunction)(int))
  {
    registeredFruits.insert(std::make_pair(name, createFunction));
    return true;
  }
  Fruit* createFruit(const std::string& name, int count)
  {
    return registeredFruits[name](count);
  }
};

//factory.cpp
Factory* Factory::factory = NULL;

//apple.h
class Apple : public Fruit
{
  static Fruit* create(int count) { return new Apple(count); }
  Apple(int count) : Fruit(count) {}
  virtual void show() { printf("%d nice apples\n", count); };  
  static bool registered;
};

// apple.cpp
bool Apple::registered = Factory::instance().registerFruit("apple", Apple::create);

//banana.h
class Banana : public Fruit
{
  static Fruit* create(int count) { return new Banana(count); }
  Banana(int count) : Fruit(count) {}
  virtual void show() { printf("%d nice bananas\n", count); };  
  static bool registered;
};

// banana.cpp
bool Banana::registered = Factory::instance().registerFruit("banana", Banana::create);

// main.cpp
int main(void)
{
  std::vector<Fruit*> fruits;
  fruits.push_back(Factory::instance().createFruit("apple", 10));
  fruits.push_back(Factory::instance().createFruit("banana", 7));
  fruits.push_back(Factory::instance().createFruit("apple", 6));
  for (size_t i = 0; i < fruits.size(); i++)
    {
      fruits[i]->show();
      delete fruits[i];
    }
  return 0;
}

Ok, this code looks fancy and it works, but here comes the but:

The C++ standard doesn't allow me to define the order in which global (static) variables will be defined.

I have 3 static variables here

Apple::registered;
Banana::registered;
Factory::factory;

The Factory::factory pointer needs to be defined to NULL before the Apple(or Banana)::registered variable, or the Factory::instance method will work with uninitialized value, and behave unpredictably.

So, what am I not getting here? Is the code really working only by an accident? If so, how should I solve the issue?

like image 435
kovarex Avatar asked Dec 03 '22 11:12

kovarex


2 Answers

All global POD data is guaranteed to be initialized to a constant value before any initializers run.

So at the start of your program, before any of the register calls are made and before main is run, the pointer is NULL and all of the bools are false, automatically. Then the initializers run, including your register calls.

Edit: Specifically, from the standard (3.6.2.2: Initialization of non-local objects):

Together, zero-initialization and constant initialization are called static initialization; all other initialization is dynamic initialization. Static initialization shall be performed before any dynamic initialization takes place.

like image 64
SoapBox Avatar answered Jan 04 '23 22:01

SoapBox


All static variables are initialized before the program begins to run. They are set at compile time and baked right into the executable.

The only issue arises when one static variable depends on another:

In a.hpp:

static int a = 1;

in b.hpp:

extern int a;
static int b = a;

The order in which static variables are initialized is not well defined, so b may or may not be 1 in this example. As long as your variables don't depend on each other, you're fine. Furthermore, is you don't give an initial value, static members are set to zero by default.

like image 32
JoshD Avatar answered Jan 04 '23 23:01

JoshD