Take the 2-minute tour ×

enter image description here

On this image you see some constants in source class that they help the factory to make the right object. I have been told that this is an anti-pattern and I should move those constants to another class. Should I move them to a factory class?

Here is the code for Factory class:

class Factory
        public function make($format)
                switch ($format)
                        case Source::Assocs:
                                return new SourceFormatsAssocs();
                        case Source::XML:
                                return new SourceFormatsXML();
                        //Some more formats
share|improve this question
add comment

2 Answers

This indeed prevents adding a new subclass without modifying the base class (to add a new constant). I would indeed move it to the factory. The factory must need to know about all the kinds of subclasses it can create. The base class doesn't need to know about its subclasses.

Moreover, these constants are only used by the factory (and the callers of the factory). The base class doesn't use them at all.

share|improve this answer
You are right - but other classes are also using those types. What do u think of crearting those consts in a class called \Formats\Types ? –  Mohammad Rahmani Feb 9 '13 at 7:00
That would also be acceptable. It seems to me that the main use of these constants is to pass them to the factory in order to choose an appropriate subclass, so putting them in the factory makese sense to me. –  JB Nizet Feb 9 '13 at 8:46
look I want to put all those files in my library which is called Sol, using namespaces like \Sol\Data\Source\Source and \Sol\Data\Source\Formats\Assocs(or Xml,Object) and \Sol\Data\Source\Formats\Factory. Now I am thinking of the client coder, if he want to add another format, he does not have access to Sol So he has to extend the factory in a way that he give the path to file containing that format class and also he has to extend \Sol\Data\Source\Formats\Types in on his own library ? Is that logical ? –  Mohammad Rahmani Feb 9 '13 at 9:10
It's now time to choose a real programming language. The best solution depends on what language you use. But you should use composition instead of inheritance to implement your extensible factory. –  JB Nizet Feb 9 '13 at 9:13
Add a registerSubclassFactory() method to the factory, which allows the client code to plug in a new factory allowing to create a new subclass instance using a new key. Have the base factory delegate to its SubclassFactories. –  JB Nizet Feb 9 '13 at 9:21
show 2 more comments

A better idea would be to consider a builder. Why? Because you can have an abstract class that knows a Formats type, and then concrete Builders that construct the various subtypes. Mind you, Builder is supposed to be done when construction involves making something complex in a number of steps, but there are variants of the builder pattern, like the static builder in Bloch and the fluent builder, where you use the pattern to chain calls to make the code more readable, and the product is generated inline.

Another possibility would be a Factory Method. This might be a good match if you are queasy about whether your class merits a Builder.

share|improve this answer
add comment

Your Answer


