views:

211

answers:

8

Hello all,

I'm applying the Factory design pattern in my C++ project, and below you can see how I am doing it. I try to improve my code by following the "anti-if" campaign, thus want to remove the if statements that I am having. Any idea how can I do it?

typedef std::map<std::string, Chip*> ChipList;

Chip* ChipFactory::createChip(const std::string& type) {
    MCList::iterator existing = Chips.find(type);
    if (existing != Chips.end()) {
        return (existing->second);
    }
    if (type == "R500") {
        return Chips[type] = new ChipR500();
    }
    if (type == "PIC32F42") {
        return Chips[type] = new ChipPIC32F42();
    }
    if (type == "34HC22") {
        return Chips[type] = new Chip34HC22();
    }
    return 0;
}

I would imagine creating a map, with string as the key, and the constructor (or something to create the object). After that, I can just get the constructor from the map using the type (type are strings) and create my object without any if. (I know I'm being a bit paranoid, but I want to know if it can be done or not.)

A: 

You can have ifs in a factory - just don't have them littered throughout your code.

Skilldrick
+1  A: 

The point of the factory is not to get rid of the ifs, but to put them in a separate place of your real business logic code and not to pollute it. It is just a separation of concerns.

Petar Minchev
A: 
struct Chip{
};

struct ChipR500 : Chip{};

struct PIC32F42 : Chip{};

struct ChipCreator{
   virtual Chip *make() = 0;
};

struct ChipR500Creator : ChipCreator{
   Chip *make(){return new ChipR500();}
};

struct PIC32F42Creator : ChipCreator{
   Chip *make(){return new PIC32F42();}
};

int main(){
   ChipR500Creator m;  // client code knows only the factory method interface, not the actuall concrete products
   Chip *p = m.make();
}
Chubsdad
+1  A: 

No you cannot get rid of the ifs. the createChip method creats a new instance depending on constant (type name )you pass as argument. but you may optimaze yuor code a little removing those 2 line out of if statment.

 microcontrollers[type] = newController;
 return microcontrollers[type];
Arseny
ah yes I will update that
phunehehe
DeadMG
+4  A: 

You are right, you should use a map from key to creation-function. In your case it would be

typedef Chip* tCreationFunc();
std::map<std::string, tCreationFunc*> microcontrollers;

for each new chip-drived class ChipXXX add a static function:

static Chip* CreateInstance()
{
    return new ChipXXX();
}

and also register this function into the map.

Your factory function should be somethink like this:

Chip* ChipFactory::createChip(std::string& type)
{
    ChipList::iterator existing = microcontrollers.find(type);    
    if (existing != microcontrollers.end())
        return existing->second();

    return NULL;
}

Note that copy constructor is not needed, as in your example.

Lior Kogan
I have trouble following your example. Where's the map of function pointers used? And also why is not a copy ctor needed?
celavek
The map is used in the createChip() function, to check if we have a Creation-function for the given string. If so, we just call that function, which return a pointer to a new object. Since we return this pointer, we don't need to copy the object.
Lior Kogan
thanks, but then how do you put the `CreateInstance` fucntion into the map?
phunehehe
ok I figured it out `creators["R500"] = ChipR500::createInstance;`
phunehehe
+2  A: 

If you're desperate, you could write a jump table/clone() combo that would do this job with no if statements.

class Factory {
    struct ChipFunctorBase {
        virtual Chip* Create();
    };
    template<typename T> struct CreateChipFunctor : ChipFunctorBase {
        Chip* Create() { return new T; }
    };
    std::unordered_map<std::string, std::unique_ptr<ChipFunctorBase>> jumptable;
    Factory() {
        jumptable["R500"] = new CreateChipFunctor<ChipR500>();
        jumptable["PIC32F42"] = new CreateChipFunctor<ChipPIC32F42>();
        jumptable["34HC22"] = new CreateChipFunctor<Chip34HC22>();
    }
    Chip* CreateNewChip(const std::string& type) {
        if(jumptable[type].get())
            return jumptable[type]->Create();
        else
            return null;
    }
};

However, this kind of approach only becomes valuable when you have large numbers of different Chip types. For just a few, it's more useful just to write a couple of ifs.

Quick note: I've used std::unordered_map and std::unique_ptr, which may not be part of your STL, depending on how new your compiler is. Replace with std::map/boost::unordered_map, and std::/boost::shared_ptr.

DeadMG
+1  A: 

To answer your question: Yes, you should make a factory with a map to functions that construct the objects you want. The objects constructed should supply and register that function with the factory themselves.

There is some reading on the subject in several other SO questions as well, so I'll let you read that instead of explaining it all here.

http://stackoverflow.com/questions/1741178/generic-factory-in-c

http://stackoverflow.com/questions/582331/c-is-there-a-way-to-instantiate-objects-from-a-string-holdig-their-class-name

Nubsis
A: 

What you are asking for, essentially, is called Virtual Construction, ie the ability the build an object whose type is only known at runtime.

Of course C++ doesn't allow constructors to be virtual, so this requires a bit of trickery. The common OO-approach is to use the Prototype pattern:

class Chip
{
public:
  virtual Chip* clone() const = 0;
};

class ChipA: public Chip
{
public:
  virtual ChipA* clone() const { return new ChipA(*this); }
};

And then instantiate a map of these prototypes and use it to build your objects (std::map<std::string,Chip*>). Typically, the map is instantiated as a singleton.

The other approach, as has been illustrated so far, is similar and consists in registering directly methods rather than an object. It might or might not be your personal preference, but it's generally slightly faster (not much, you just avoid a virtual dispatch) and the memory is easier to handle (you don't have to do delete on pointers to functions).

What you should pay attention however is the memory management aspect. You don't want to go leaking so make sure to use RAII idioms.

Matthieu M.