Reputation: 5052
I was reading up on factory methods. Can someone explain why it is suggested that factory methods be located in a separate factory class?
I am pulling my example from here: http://www.devshed.com/c/a/PHP/Design-Patterns-in-PHP-Factory-Method-and-Abstract-Factory/
class ETF {
var $data;
function __construct($data) {
$this->data = $data;
}
function process() {}
function getResult() {}
}
class VirtualCheck extends ETF {}
class WireTransfer extends ETF {}
class ETFFactory {
function createETF($data) {
switch ($data[‘etfType’]) {
case ETF_VIRTUALCHECK :
return new VirtualCheck($data);
case ETF_WIRETRANSFER :
return new WireTransfer($data);
default :
return new ETF($data);
}
}
}
$data = $_POST;
$etf = ETFFactory::createETF($data);
$etf->process();
I would tend to instead write it like this:
class ETF {
final public static function factory($data) {
switch ($data[‘etfType’]) {
case ETF_VIRTUALCHECK :
return new VirtualCheck($data);
case ETF_WIRETRANSFER :
return new WireTransfer($data);
default :
return new ETF($data);
}
}
var $data;
function ETF($data) {
$this->data = $data;
}
function process() {}
function getResult() {}
}
class VirtualCheck extends ETF {}
class WireTransfer extends ETF {}
$data = $_POST;
$etf = ETF::factory($data);
$etf->process();
Am I wrong in doing this?
Upvotes: 5
Views: 1251
Reputation: 4905
I suspect there is some misusage of Factory Method and Abstract Factory terms.
Factory Method allows us to specify interface of some object which we expect to use in the code but don't know its implementation at the moment or we know only some default implementation which can be different in some cases. So we leave this decision to subclasses. Another possible usage is hiding object creation details or defining a constructor-like method for simpler usage.
Abstract Factory provides an interface for creating families of related objects. We use it when want to isolate the whole creation process from the usage. For example, I used it when developed several different turn-based card games. All games were different and had different logic (diff number of players, diff winning combinations, the order of turns, point system, etc). But since they were turn-based card games they were in the same time the same (from diff angle): you wait for enough amount of players (each player object is diff for each game), you have a dealer (another object which encapsulates rules and is diff for each implementation) and you run the game just switching turns between players until some condition happens. So I was able to use the same gaming framework which I initialized with diff abstract factory implementation.
Regarding your question. In your case you have a final static constructor-like method. Technically it violates single responsibility principle but I would say it is rather related to discussion "should we call static methods on instances". If we don't call static methods on instances it violates nothing. In my opinion, it is a valid usage of factory method and you won't have any problems with this code.
Useful links:
Upvotes: 0
Reputation: 39364
I would not say you're "wrong", but there is a "smell". By combining the factory method in the manufactured class, the architecture violates a few of the SOLID guidelines:
I've found that the more SOLID a class is, the easier the class is to maintain in the long-term. Thus I wouldn't consider SOLID violations immediate problems, just a signal of possible trouble down the line.
So, what trouble might you run into down the line? Well, if the factory itself becomes more complex, you'll need methods to handle that additional work. These methods would not, necesasrily, be used by the class proper methods. So you'd end up with code like this:
class ETF {
final public static factory($kind) {
switch ($kind) {
case 'A':
$etf = static::factoryHelperForA();
break;
case 'B':
$etf = static::factoryHelperForA();
break;
}
return $etf;
}
public function apiMethod1() {
$this->apiMethod1Helper();
}
public function apiMethod2() {
$this->apiMethod2Helper();
}
// factory helper
private static function factoryHelperForA() {
/* lots of code */
}
// another factory helper
private static function factoryHelperForB() {
/* lots of code */
}
// ugh, now we have api method helpers... totally different responsibility
private function apiMethod1Helper() {
/* lots of code */
}
// still more...
private function apiMethod2Helper() {
/* lots of code */
}
}
So you can see it starts to become messy as the needs of the factory grow, and as the needs of the manufactured class grow. This is what the SOLID principles are guiding you away from.
If it's important to you now to build a future flexible class, then I'd suggest busting the factory out into its own EtfFactory
class.
Upvotes: 4