Is a "factory" method the right pattern?

Question

So I'm working to improve an existing implementation. I have a number of polymorphic classes that are all composed into a higher level container class. The problem I'm dealing with at the moment is that the higher level container class, well, sucks. It looks something like this, which I really don't have a problem with (as the polymorphic classes in the container should be public). My real issue is the constructor...

/* * class1 and class 2 derive from the same superclass */ class Container { public: boost::shared_ptr<ComposedClass1> class1; boost::shared_ptr<ComposedClass2> class2; private: ... } /* * Constructor - builds the objects that we need in this container. */ Container::Container(some params) { class1.reset(new ComposedClass1(...)); class2.reset(new ComposedClass2(...)); }

What I really need is to make this container class more re-usable. By hard-coding up the member objects and instantiating them, it basically isn't and can only be used once. A factory is one way to build what I need (potentially by supplying a list of objects and their specific types to be created?) Other ways to get around this problem? Seems like someone should have solved it before... Thanks!

Any details about what the container is supposed to do? Is it trying to reimplement polymorphism? Will the pointers be reset to some other objects later? — Potatoswatter
– Potatoswatter, Commented Mar 11, 2010 at 4:12
no, not reimplementing polymorphism. It's just a container for a number of "components". In general, a set of components do something interesting. You can form a different container with a different set of components and do something else interesting. There would be no reason to reset the pointers AFAIK. — jdt141
– jdt141, Commented Mar 11, 2010 at 4:16
Way too abstract to give a useful answer except by chance. Are you to trying to re-use the same container instance with different class1 and class2s or are you trying to re-use a different instance of the container? Probably DI (William Billingsley's response) will be helpful. — Tom
– Tom, Commented Mar 11, 2010 at 5:00
Sorry about that Tom. In re-reading it, I agree. You would be using different instances of the container. So you would have Container container1; Container container2; and each would have different composed classes. — jdt141
– jdt141, Commented Mar 11, 2010 at 13:52

William Billingsley · Accepted Answer · 2010-03-11 04:04:58Z

3

Dependency injection springs to mind.

answered Mar 11, 2010 at 4:04

William Billingsley

7713 silver badges6 bronze badges

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

1 Comment

jdt141 Over a year ago

thanks for the link. in reading the link, I'm pretty sure this is what i'm looking for, hence you win! :-) thanks much.

Ozan · Accepted Answer · 2010-03-11 04:59:41Z

You use a factory method if you change the created type in derived classes, i.e. if descendants of Container use different descendants of class1 and class2.

class1* Container::GetClass1; virtual; { return new ComposedClass1; } class1* SpecialContainer::GetClass1; { return new SpecialComposedClass1; } Container::Container(some params) { class1.reset(GetClass1); class2.reset(GetClass2); }

A different approach is decoupling creation of Container's dependencies from Container. For starters, you can change the constructor to dependency-inject class1 and class2.

Container::Container(aclass1, aclass2) { class1.reset(aclass1); class2.reset(aclass2); }

This approach is normally fine, but you should not use factory methods in a constructor (constructors should not call virtual methods, as they are executing on a class before has finished constructing). I would recommend an abstract factory instead with dependency injection.

Tom · Accepted Answer · 2010-03-11 05:07:14Z

You do know you can just have a second constructor that doesn't do that right?

Container::Container(some params) { // any other initialization you need to do }

You can then set class1 and class2 as you please as they're public.

Of course they shouldn't be but given this code that's what you could do.

As far as I can see from what you've described the factory pattern wouldn't be that helpful.

Sure, I could do that, but the problem is that a different instance of the container may not even need Class1 and Class2 composed into it. It may have Class3, 4, and 5 as objects composed into it. Trying to find a general way.

iain · Accepted Answer · 2010-03-11 08:22:41Z

Based on your description I would recommend an Abstract Factory passed into the constructor.

class ClassFactory { public: virtual ~ClassFactory virtual Class1 * createClass1(...) = 0; // obviously these are real params not varargs virtual Class2 * createClass2(...) = 0; }; class DefaultClassFactory : public ClassFactory { // .... }; Container::Container( some params, boost::smart_ptr<ClassFactory> factory = boost::smart_ptr<ClassFactory>(new DefaultClassFactory)) { class1.reset(factory->createClass1(...)); class2.reset(factory->createClass1(...)); }

This is a form of dependency injection without the framework. Normal clients can use the DefaultClassFactory transparently as before. Other clients for example unit tests can inject their own ClassFactory implementation.

Depending on how you want to control ownership of the factory parameter this could be a smart_ptr, scoped_ptr or a reference. The reference can lead to slightly cleaner syntax.

Container::Container( some params, ClassFactory & factory = DefaultClassFactory()) { class1.reset(factory.createClass1(...)); class2.reset(factory.createClass1(...)); } void SomeClient() { Container c(some params, SpecialClassFactory()); // do special stuff }

mpen · Accepted Answer · 2010-03-11 08:28:03Z

public class factory { public static Class1 Create() { return new Class1(); } } class Class1 {} main() { Classs1 c=factory.Create(); }

peterchen · Accepted Answer · 2010-03-11 08:41:23Z

Generally, I agree with Toms comment: not enough info.

Based on your clarification:

It's just a container for a number of "components"

If the list of components varies, provide Add/Enumerate/Remove methods on the container.

If the list of components should be fixed for each instance of the container, provide a builder object with the methods

Builder.AddComponent Builder.CreateContainer

The first would allow to reuse the container, but often ends up more complex in my experience. The second one often requires a way to swap / replace containers but ends up easier to implement.

Dependency Injection libraries can help you configure the actual types outside of the aplication (though I still need to see why this is such a great leap forward).

Collectives™ on Stack Overflow

Is a "factory" method the right pattern?

6 Answers 6

1 Comment

1 Comment

1 Comment

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

1 Comment

1 Comment

1 Comment

Comments

Comments

Comments

Related