What is the right way to implement factory pattern?

Question

I just wonder if this is the right way to create objects and implement the factory pattern in PHP. I know we have factory method pattern and Abstract factory pattern, but do we have a pattern like the following http://noondreams.com/shared/data/pages/images/Factory.png ?

class Factory { public function make($format) { switch($format) { case Source::Assocs: return new \Source\Formats\Assocs(); case Source::XML return new \Source\Formats\XML(); //Some other formats } } }

Although I would use a static function like \Source\Formats::create($format), your code looks good. Whats wrong with that? — hek2mgl
– hek2mgl, Commented Feb 8, 2013 at 21:27
This is rather an anti-pattern: long switch-statement and a parent class that knows all of its child classes — Fabian Schmengler
– Fabian Schmengler, Commented Feb 8, 2013 at 21:28
@fab That's not a generic factory, but it is ok. I see no 'anti-pattern' — hek2mgl
– hek2mgl, Commented Feb 8, 2013 at 21:29
@hek2mgl According to the image link I left in the question and of what I've learnt right now, at least one thing is anti pattern and that is "leaving the types in Source class" — Mohammad Rahmani
– Mohammad Rahmani, Commented Feb 8, 2013 at 21:38

Colin M · Accepted Answer · 2013-02-08 21:26:53Z

No, this isn't really a factory pattern. The factory pattern would look more like this:

<?php abstract class File { public static function createFromFile($filename) { $extension = /* get file extension */; switch ($extension) { case 'xml': return new XmlFile($filename); break; case 'php': return new PhpFile($filename); break; } throw new \InvalidArgumentException(); } } class XmlFile extends File { } class PhpFile extends File { }

Notice how the abstract class is creating instances of concrete classes which extend it without the user having to worry about the various types it may return.

Note: in a real scenario, you wouldn't use a switch statement, but likely reflection or various other techniques since the abstract class wouldn't know all of it's child classes.

That may look more like this:

<?php abstract class File { public static function createFromFile($filename) { $extension = /* get file extension */; $extension = ucfirst($extension); $reflection = new ReflectionClass($extension . 'File'); return $reflection->newInstanceArgs(array($filename)); } } class XmlFile extends File { } class PhpFile extends File { }

What is the major difference between my code an yours? Although it seems that in yours, the super class knows about the subclasses
@MohammadRahmani 1) As it does in yours. 2) Only in the first example, and I clarified afterwards that it was just for demonstration and real scenarios would be different. 3) The factory is the abstract class which sub classes extend. Factory patterns don't make sense if the returned object isn't of the same type as the one creating the objects.
But I think you are making the super class know about the sub classes and this is an anti pattern
In the second example, the super class does NOT know of the children.
Hi @MohammadRahmani. I think what might be throwing you off is the way the object is being created. You generally avoid making your superclass dependent on the subclasses. Colin avoided that by using reflection to find the correct subclass to instantiate. The factory pattern doesn't require that the superclass create the instances of subclasses. In fact, the "Abstract Factory Pattern" article on Wikipedia uses a separate class as the factory. That class does not use reflection -- it creates the objects directly.

Collectives™ on Stack Overflow

What is the right way to implement factory pattern?

1 Answer 1

7 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Linked

Related