Sign in to follow this  

Design question about create factory method.

This topic is 3665 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi all, Sorry for the dumb topic name. I didn´t know what exaplanation to use for the topic :). Well, I would like someone to have a look at a problem I have found several times and couldn´t been able to found a satisfying solution. Think for example of a factory template to create objects of a class, that have a Create method. We can have something like this: template <class _T> class CFactory : public CSingleton < CFactory> { _T* Create()=0; .... } The problem is that every object we create may have a different signature depending on its type, for this I got some solutions, but I´m not pretty happy with them: 1) Having a param list in the create function: template <class _T> class CFactory : public CSingleton < CFactory> { _T* Create(ParamList)=0; .... } this way all methods will have same signature and the only thing we will have is to rewrite them in child clases. This is somehow a radical decision for me. 2) Not have the Create in the generic class and move it to the children. Ummm, this will work but, I don´t have anything forcing/telling me to write the Create functions in child clases, well thet compiler will claims that it can found the method in the class if I try to call it, but seems dirty to me. if would be something like this: class CFactory : public CSingleton < CFactory> { .... } class ParticularFactory : public CFactory { ParticularObject* Create( Particular creationg signature) { ... } } So, has anybody found in the same situation and did find a better solution?. Thanks in advance, notbad.

Share this post


Link to post
Share on other sites
A factory is a black box. You feed it the key, it returns the instance.

If you need to know which type you want before you call the factory (constructor args), then you cannot use factory - at very least it won't be clean.

If you need to define in advance which parameters you need, then you don't need the factory. The whole point of a factory is that you can replace implementations return at any point (in extreme cases via run-time class instantiation and reflection).

For example - let's say your factory makes Widgets. You want it to create a widget named "ListBox". If you later replace that with "Tree", nothing changes. Or better yet, you create a widget from a string. This string will be obtained from some configuration file and may change at any point - how will you know which parameters to pass it?

The only suitable way is to provide the factory with some configuration object, which the *newly created* instance can query for parameters.

For certain types of tasks, I choose one such object. For example, when creating messages from network, I pass the Buffer into factory. Factory then reads the message ID, instantiates the proper object, and passes remaining buffer to constructor of that message, which reads relevant fields.

The only one that knows about the contents is the message itself. Neither the factory nor the user know how to populate it.

Example:

void onReceive( Buffer *buf )
{
Message *msg = factory.createInstance(buf);
msg->dispatch(target);
}

Message * Factory::createInstance(Buffer *buf)
{
switch (buf->readId()) {
case ...
case ... : return new SomeMessage(buf);
}
}

SomeMessage::SomeMessage(Buffer *buf)
: someField(buf->readString())
, anotherField(buf->readInt())
, andAnother(buf->readFloat();
{}

Share this post


Link to post
Share on other sites
Quote:
Well, this seems pretty the same that the Param List option I exposed. But params are packed in a string, don´t you think?.

I do no think so no, the factory is using the buffer to exstract the key not the class using it as parameters. In addition I also do not like the requirement of the clone/ create function when there is not a need for it.
I personally like using a method like:
http://www.liamdevine.co.uk/code/abstract_factory.php

Share this post


Link to post
Share on other sites
Quote:
Original post by notbad
Well, this seems pretty the same that the Param List option I exposed. But params are packed in a string, don´t you think?.


The important question is: Who populates the Param List?

It's the problem of encoding or signature, but where do the parameters come from?

In XML reader, you'd pass the DOM node to factory. Factory would then instantiate whatever object is needed by passing it the child DOM nodes, which in turn would call factory as needed.

The important thing is, neither you nor the factory have any knowledge of what the parameters are, nor may they be dependent on the type of object being instantiated, unless that instance is implied by the parameters themselves.

Share this post


Link to post
Share on other sites
Well Antheus, for me, factory should return an initialized instance of the type I asked for. So, I need the type of object to construct, and I know what params will be used, so I will be populating the list.


notbad.

Share this post


Link to post
Share on other sites
Quote:
Original post by notbad
Well Antheus, for me, factory should return an initialized instance of the type I asked for. So, I need the type of object to construct, and I know what params will be used, so I will be populating the list.


In that case you don't need a factory, just create the instances. By reverting to factory pattern, you're merely creating another redundant abstraction.

Factories are used when you don't know which object you'll be creating.

Think of it this way: implementation of a proper factory can, at any given time, be replaced. Either with no members, with new members, or by changing existing members - yet the code that depends on it doesn't change, in cases of platforms that support it - it can be done during run-time, requiring no recompilation of the rest of application.

If you know not only the type of object to create, but parameters to pass to it, just do it directly - factory won't do its primary function, which is to serve as an abstraction.

This is analogous to asking the type of polymorphic classes. If you need to know of which type a certain instance is, there's something wrong with design, and introduces coupling which polymorphism should prevent.

Share this post


Link to post
Share on other sites
I see... So I should be going with the "moving create method to child classes", Or just forgetting about a base class that will be inherited from child, and write final classes directly, am I ok? I just was trying to use the factory thing to reuse code, because base class will have a base structure I would like to spread through children. So, I´m left only with the first method (create method only in children), do you think it is ok?.

Note: Thanks a lot for the fast asnwers.

notbad.

Share this post


Link to post
Share on other sites
Quote:
"moving create method to child classes"


I don't quite get this.

A factory is a single method/function, which takes a key, and returns an instance of an object:

BaseType *create(int key)
{
switch (key) {
case 0 : return new Foo();
case 1 : return new Bar();
case 2 : return new FooBar();
}
return NULL;
}



There are various ways to make this more elaborate, but classes returned know nothing of factory, not even that this is how they are instantiated.

Depending on language, there are obviously variations on that, more or less elaborate, but if you're introducing factory-related methods into classes returned by factory there's usually something wrong.

There really is nothing more about factories than a single method. No need for singletons or anything as such. Unless you have dynamic factories, what does it matter how many instances there are. They hold no state.


If you're tempted to use some form of auto-registration, I'd suggest against it. C++ isn't good for that, and you'll run into problems, no matter how you try to enforce the single instance.

Look into the link provided above which demonstrates abstract factories. Then, at start, explicitly register the instances you want returned. There's also a more comprehensive overview at codeproject.

Share this post


Link to post
Share on other sites
Yes, I understood that part (the factory one), they are used to instantiate just objects, they don´t know anything about the nature of the object it instantiates , just instantation is its duty.

Ok, but now think on a system like this:

System
{
...Logic and data...
}

I will have some types of systems that I want to be "factories" for its own type of objects:

RenderSytem
{
RenderObject* Create( MeshFile, ...);
}

IASystem
{
IAObject* Create(Script);
}

What I was asking before was if a good solution to reuse the logic/data of System in children subsystems (RenderSystem, IASystem), coudl be to move the Create methods to children classes, this way, in this classes I really know what params an RenderObject or a IAObject will use to initialize, and I still have all functionality/data inherited from parent.

Thanks in advance, and sorry for the long post,
notbad.

Share this post


Link to post
Share on other sites
If I understand what you're asking, it's possible, but not a particularly good idea.

#include <boost/smart_ptr.hpp>
#include <boost/mpl/list.hpp>
#include <boost/function.hpp>
#include <typeinfo>
#include <map>
#include <iostream>

using namespace boost;

class TypeInfoProxy {
public:
TypeInfoProxy(const std::type_info * t) : ti(t) {}
const std::type_info * ti;
};

inline bool operator<(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return lhs.ti->before(*rhs.ti) != 0;
}

inline bool operator>(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return rhs.ti->before(*lhs.ti) != 0;
}

inline bool operator<=(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return !(rhs > lhs);
}

inline bool operator>=(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return !(rhs < lhs);
}

inline bool operator==(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return ((*lhs.ti) == (*rhs.ti)) != 0;
}

inline bool operator!=(const TypeInfoProxy & lhs, const TypeInfoProxy & rhs) {
return ((*lhs.ti) != (*rhs.ti)) != 0;
}

template <typename T, typename TokenType>
class ICreator {
public:
virtual shared_ptr<T> create(void * args) = 0;
virtual ~ICreator() {}
};

template <typename T>
struct Package {};

template <typename U1>
struct Package<mpl::list<U1> > {
U1 arg1;
};

template <typename U1, typename U2>
struct Package<mpl::list<U1, U2> > {
U1 arg1;
U2 arg2;
};

template <typename T, typename TypeList, typename TokenType>
class ConcreteCreator : public ICreator<T, TokenType> {};

template <typename T, typename TokenType>
class ConcreteCreator<T, mpl::list<>, TokenType> : public ICreator<T, TokenType> {
public:
typedef function<shared_ptr<T> (void)> FunctorType;
ConcreteCreator(FunctorType functor) : functor_(functor) {}

virtual shared_ptr<T> create(void *) {
return functor_();
}
virtual ~ConcreteCreator() {}
private:
FunctorType functor_;
};

template <typename T, typename TokenType, typename U1>
class ConcreteCreator<T, mpl::list<U1>, TokenType> : public ICreator<T, TokenType> {
public:
typedef function<shared_ptr<T> (U1)> FunctorType;
typedef Package<mpl::list<U1> > PackageType;

ConcreteCreator(FunctorType functor) : functor_(functor) {}

virtual shared_ptr<T> create(void * ptr) {
PackageType * package = reinterpret_cast<PackageType *>(ptr);
return functor_(package->arg1);
}
virtual ~ConcreteCreator() {}
private:
FunctorType functor_;
};

template <typename T, typename TokenType, typename U1, typename U2>
class ConcreteCreator<T, mpl::list<U1, U2>, TokenType> : public ICreator<T, TokenType> {
public:
typedef function<shared_ptr<T> (U1, U2)> FunctorType;
typedef Package<mpl::list<U1, U2> > PackageType;

ConcreteCreator(FunctorType functor) : functor_(functor) {}

virtual shared_ptr<T> create(void * ptr) {
PackageType * package = reinterpret_cast<PackageType *>(ptr);
return functor_(package->arg1, package->arg2);
}
virtual ~ConcreteCreator() {}
private:
FunctorType functor_;
};


template <typename T, typename TokenType>
class Factory {
public:
shared_ptr<T> create(TokenType type_token) {
KeyType key = std::make_pair(type_token, &typeid(void));
CreatorMap::iterator itr = creator_map_.find(key);
if (itr == creator_map_.end())
throw std::runtime_error("No registered creator");
return (itr->second)->create(0);
}

template <typename U1>
shared_ptr<T> create(TokenType type_token, U1 a1) {
KeyType key = std::make_pair(type_token, &typeid(mpl::list<U1>));
CreatorMap::iterator itr = creator_map_.find(key);
if (itr == creator_map_.end())
throw std::runtime_error("No registered creator");
Package<mpl::list<U1> > package = { a1 };
return (itr->second)->create(&package);
}

template <typename U1, typename U2>
shared_ptr<T> create(TokenType type_token, U1 a1, U2 a2) {
KeyType key = std::make_pair(type_token, &typeid(mpl::list<U1, U2>));
CreatorMap::iterator itr = creator_map_.find(key);
if (itr == creator_map_.end())
throw std::runtime_error("No registered creator");
Package<mpl::list<U1, U2> > package = { a1, a2 };
return (itr->second)->create(&package);
}

void register_creator(TokenType type, function<shared_ptr<T> (void)> func) {
CreatorPtr ptr(new ConcreteCreator<T, mpl::list<>, TokenType>(func));
KeyType key = std::make_pair(type, &typeid(void));
creator_map_[key] = ptr;
}

template <typename U1>
void register_creator(TokenType type, function<shared_ptr<T> (U1)> func) {
CreatorPtr ptr(new ConcreteCreator<T, mpl::list<U1>, TokenType>(func));
KeyType key = std::make_pair(type, &typeid(mpl::list<U1>));
creator_map_[key] = ptr;
}

template <typename U1, typename U2>
void register_creator(TokenType type, function<shared_ptr<T> (U1, U2)> func) {
CreatorPtr ptr(new ConcreteCreator<T, mpl::list<U1, U2>, TokenType>(func));
KeyType key = std::make_pair(type, &typeid(mpl::list<U1, U2>));
creator_map_[key] = ptr;
}
private:
typedef std::pair<TokenType, TypeInfoProxy> KeyType;
typedef shared_ptr<ICreator<T, TokenType> > CreatorPtr;
typedef std::map<KeyType, CreatorPtr> CreatorMap;
CreatorMap creator_map_;
};

struct Shape {
virtual void print(void) = 0;
virtual ~Shape() {}
};

typedef shared_ptr<Shape> ShapePtr;

struct Square : Shape {
Square(int side) : side(side) {}
void print(void) { std::cout << "I'm a square of size " << side << "!\n"; }

static ShapePtr creator(int s) { return ShapePtr(new Square(s)); }

int side;
};

struct Rectangle : Shape {
Rectangle(int h, int w) : h(h), w(w) {}
void print(void) { std::cout << "I'm a rectangle of sides " << h << " and " << w << "!\n"; }

static ShapePtr creator(int h, int w) { return ShapePtr(new Rectangle(h, w)); }

int h, w;
};

int main(int, char **) {
Factory<Shape, int> shape_factory;
shape_factory.register_creator(0, function<ShapePtr (int)>(&Square::creator));
shape_factory.register_creator(1, function<ShapePtr (int, int)>(&Rectangle::creator));

ShapePtr square = shape_factory.create(0, 5);
square->print();

ShapePtr rect = shape_factory.create(1, 3, 4);
rect->print();

return 0;
}


This should compile and run on a reasonably recent compiler with boost 1.34.1 installed. Take a look at the creation calls at the end. Now change the second call from shape_factory.create(1, 3, 4) to shape_factory.create(1, 3, 4.0) It will now magically throw an exception because it doesn't know how to handle the double, since the creation handler was installed as (int, int).

Share this post


Link to post
Share on other sites
Hehe sicrane, nice stuff, but somehow dificult for me :). I think I will go for waht I said before, I will move my create function to concrete children clases, this way, I know the signature, and can create just the type of objects I need.

Perhaps it is just dirty and ugly, but for now I think is a good option.

Thanks for all the answers,
HexDump.

Share this post


Link to post
Share on other sites

This topic is 3665 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this