Is a "factory" method the right pattern?

Question

Hey all -

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!

Answer 1

Dependency injection springs to mind.

Answer 2

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);
}

Answer 3

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.

Answer 4

public class factory
{
    public static Class1 Create()
      {
            return new Class1();
      }
}

class Class1
{}
main()
{
   Classs1 c=factory.Create();   
}

Answer 5

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
}

Answer 6

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).