Question

My question is more or less identical to the one at Need a design pattern to remove enums and switch statement in object creation However I don't see that the abstract factory pattern suits well here.

I'm currently planning the refactoring/reimplementation of some existing DAL/ORM mixture library. Somewhere in the existing code there is code that looks like this:

class Base
{
  static Base * create(struct Databasevalues dbValues)
  {
    switch(dbValues.ObjectType)
    {
    case typeA:
      return new DerivedA(dbValues);
      break;
    case typeB:
      return new DerivedB(dbValues);
      break;
    }
  }
}

class DerivedA : public Base
{
  // ...
}

class DerivedB : public Base
{
  // ...
}

So the library responsible for database communication populates a struct with all information about the database entity and then the above create() method is called to actually create the corresponding object in the ORM. But I don't like the idea of a base class knowing of all its derived classes and I don't like the switch statement either. I also would like to avoid creating another class just for the purpose of creating those Objects. What do you think about the current approach? How would you implement this functionality?

Was it helpful?

Solution 3

No matter what you do, you'll need either switch-case or some other construct that will just hide similar logic.

What you can and should do, however, is remove the create method from your Base - you're totally correct it shouldn't be aware of it's derived ones. This logic belongs to another entity, such as factory or controller.

OTHER TIPS

This has been discussed here milliions of times. If you don't want to create a separate factory class, you can do this.

class Base
{

public:
    template <class T>
    static void Register  (TObjectType type)
    {
       _creators[type] = &creator<T>;
    }

    static Base* Create (TObjectType type)
    {
        std::map <TObjectType, Creator>::iterator C = _creators.find (type);
        if (C != _creators.end())
          return C->second ();

        return 0;        
    }

private:
    template <class T>
    static Base* creator ()
    {
        return new T;
    }

private:
    typedef Base* (::*Creator) ();
    static std::map <TObjectType, Creator> _creators;
};

int main ()
{
   Base::Register <Derived1> (typeA);
   Base::Register <Derived2> (typeB);

   Base* a = Base::Create (typeA);
   Base* b = Base::Create (typeB);
}

Let's say you replace the switch with a mapping, like map<ObjectType, function<Base* (DatabaseValues&)>>.

Now, the factory (which may or may not live in the base class), doesn't need to know about all the subclasses.

However, the map has to be populated somehow. This means either something populates it (so your knowing about all subclasses problem has just been pushed from one place to another), or you need subclasses to use static initialization to register their factory functions in the map.

Just don't use enums. They are not OO construction, that was why JAVA did not have them at the beginning (unfortunately the pressure was too big to add them).

Consider instead of such enum:

enum Types {
  typeA,
  typeB
};

this construction, which do not need switch (another non OO construction in my opinion) and maps:

Types.h

class Base;
class BaseFactory {
public:
   virtual Base* create() = 0;
};     
class Types {
public:
  // possible values 
  static Types typeA;
  static Types typeB;
  // just for comparison - if you do not need - do not write...
  friend bool operator == (const Types & l, const Types & r) 
  { return l.unique_id == r.unique_id; }
  // and make any other properties in this enum equivalent - don't add them somewhere else 
  Base* create() { return baseFactory->create(); }

private:
   Types(BaseFactory* baseFactory, unsigned unique_id);
   BaseFactory* baseFactory;
   unsigned unique_id; // don't ever write public getter for this member variable!!!
};   

Types.cpp

#include "Types.h"
#include "Base.h"
#include "TypeA.h"
#include "TypeB.h"

namespace {
  TypeAFactory typeAFactory;
  TypeBFactory typeAFactory;
   unsigned unique_id = 0;
}
Types Types::typeA(&typeAFactory, unique_id++);
Types Types::typeA(&typeBFactory, unique_id++);

So your example (if you really would need this function then):

class Base
{
  static Base * create(struct Databasevalues dbValues)
  {
    return dbValues.ObjectType.create();
  }
};

Missing parts should be easy to implement.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top