4

I have a factory class to build objects of base class B. The object (D) that uses this factory receives a list of strings representing the actual types. What is the correct implementation:

  1. the factory receives an Enum (and uses switch inside the Create function) and D is responsible to convert the string to Enum.
  2. the factory receives a string and checks for a match to a set of valid strings (using ifs')
  3. other implementation i didn't think of.

5 Answers 5

1

I would separate the conversion of strings to enum into a distinct object. This can easily be solved by a map btw. But error handling etc. is still something which neither D nor the factory should be worried about.

Then either D calls the converter to get its enum, or it is already converted beforehand, so D only needs to pass the enum to the factory. (Btw the factory would better use a map too instead of a switch internally).

This raises the question: do you actually need the enums at all (in places other than D and the factory)? If not, maybe the enum could be left out of the picture and you could use a map to convert directly from strings to types (i.e. - since C++ doesn't support dynamic class loading - to function objects which create the necessary concrete type instances for you). A rough example (I don't have an IDE to test it so bear with me if there are any errors in it):

// Function type returning a pointer to B typedef (B*)(*func)() StaticConstructor; // Function creating instances of subclass E B* createSubclassE() { return new E(...); } // Function creating instances of subclass F B* createSubclassF() { return new F(...); } // Mapping from strings to constructor methods creating specific subclasses of B map<string, StaticConstructor> factoryMap; factoryMap["E"] = &createSubclassE; factoryMap["F"] = &createSubclassF; 

Of course, the created instances should also be disposed of properly - in production code, the returned objects could be e.g. enclosed in an auto_ptr. But I hope this short example is enough to show you the basic idea. Here is a tutorial if you want more...

Sign up to request clarification or add additional context in comments.

1 Comment

what do you mean by string to type map? how that map should look like?
0

You can put all matching strings in the set or list and check if it contains your strings instead of writing ifs/switches.

3 Comments

The factory still have to construct the correct object according to the string. It has to do ifs/switches
It is possible to have map of string -> creator function which is much better than ifs/switches.
I think that switching Enum is inevitable. You will do that anyway, explicitly or implicitly.
0

My project on VC++/Qt had a large number of XML files containing strings that had a Enum representation into the source.

So for each Enum we had a wrapper with overloaded operator QString <> Enum:

enum DataColumnTypeEnum { DataColumnTypeNotSet, ColumnBinary, ColumnBoolean, ColumnDate, ColumnDateTime, ColumnNumber, ColumnFloat, ColumnPrimary, ColumnString, ColumnText, }; class DataColumnType { public: DataColumnType(); DataColumnType(DataColumnTypeEnum); DataColumnType(const QString&); DataColumnType& operator = (DataColumnTypeEnum); DataColumnType& operator = (const QString&); operator DataColumnTypeEnum() const; operator QString() const; private: DataColumnTypeEnum type; }; DataColumnType& DataColumnType::operator = (const QString& str) { str.toLower(); if(str.isEmpty()) type = DataColumnTypeNotSet; else if(str == "binary") type = ColumnBinary; else if(str == "bool") type = ColumnBoolean; else if(str == "date") type = ColumnDate; else if(str == "datetime") type = ColumnDateTime; else if(str == "number") type = ColumnNumber; else if(str == "float") type = ColumnFloat; else if(str == "primary") type = ColumnPrimary; else if(str == "string") type = ColumnString; else if(str == "text") type = ColumnText; return *this; } 

but the approach in last listing is very ugly.

Better to create a static hash table or dictionary and look up into.

Comments

0

The normal way is to have your factory as a singleton. Then each class based on class B registers it's create function and name with the factory at static initialisiation time. This is often done with macros. The factory then can creates a fast hash table of these name to create functions. Etc... you get the drift.

2 Comments

I disagree. A factory is not necessarily a Singleton at all. In fact, since Singletons make unit testing difficult, they are best avoided unless really necessary.
Thanks Péter (+1 on the comment). I agree the factory itself doesn't need to be a singleton but the list of classes has to exist somehow and I prefer to contruct it with the static initialisation. Instead of within a function or even a data file. When working in large teams it prevents people from fighting other that file. Other methods can be more desirable depending on needs.
0

I personally use an enhanced enum because I've always found the enum of C++ lacking: messages like Type 3 - method -begin aren't much informative.

To this way, I use a simple templated class:

template <class Holder> class Enum { public: typedef typename Holder::type enum_type; Enum(): mValue(Invalid()) {} Enum(enum_type i): mValue(Get(i)) {} explicit Enum(const std::string& s): mValue(Get(s)) {} bool isValid() const { return mValue != Invalid(); } enum_type getValue() const { return mValue->first; } private: typedef typename Holder::mapping_type mapping_type; typedef typename mapping_type::const_iterator iterator; static const mapping_type& Mapping() { static mapping_type MMap = Holder::Initialize(); return MMap; } static iterator Invalid() { return Mapping().end(); } static iterator Get(enum_type i) { // search } static iterator Get(const std::string& s) { // search } iterator mValue; }; 

You define Holder like so:

struct Example { typedef enum { Value1, Value2, Value3 } type; typedef std::vector< std::pair< type, std::string > > mapping_type; static mapping_type Initialize() { return builder<mapping_type>()(Value1,"Value1")(Value2,"Value2")(Value3,"Value3"); } }; 

You can define a macro for it:

DEFINE_ENUM(Example, (Value1)(Value2)(Value3)) 

But I let the implementation as an exercise (Boost.Preprocessor is your friend).

The cool thing is to use it!

int main(int argc, char* argv[]) { std::string s; std::cin >> s; Enum<Example> e(s); switch(e.getValue()) { case Example::Value1: case Example::Value2: ++e; case Example::Value3: std::cout << e << std::endl; default: } } 

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.