patternphpMinor
Is this a fair use for a factory pattern?
Viewed 0 times
thisfairfactoryforusepattern
Problem
class AcmeFactory
{
public static function anvil()
{
$crud = new Crud;
$productTypes = new ProductTypes($crud);
$productImages = new ProductImages($crud);
$product = new Anvils ($crud, $productImages);
$productManager = new ProductManager ($productTypes, $product);
return $productManager;
}
public static function jetPack()
{
$crud = new Crud;
$productTypes = new ProductTypes($crud);
$productImages = new ProductImages($crud);
$product = new JetPacks ($crud, $productImages);
$productManager = new ProductManager ($productTypes, $product);
return $productManager;
}
}I know the above code works, but am I using the factory pattern correctly?
The intended result is this:
$anvil = new AcmeFactory::anvil();
$anvil->create($weight);
$anvil->display($id);
.....Solution
Rather than doing it that way (separate methods for separate configurations) you should pass in the "configuration" to a single method and let that one handle it.
You can of course extract the choice of product into a separate (private) function again ;)
then you'd use this as follows:
I am not that good with PHP, but I think you should somehow limit what you allow to be passed into your create function. In Java I'd solve this using an enum, but as I said, I don't know enough php :(
Apart from that, I have a minor nitpick too:
Be consistent in your use of whitespaces before parentheses. Compare:
class AcmeFactory
{
public static function create($config)
{
$crud = new Crud;
$productTypes = new ProductTypes($crud);
$productImages = new ProductImages($crud);
$product = null;
if($config == "anvil")
{
$product = new Anvils($crud, $productImages);
}
else if($config == "jetpack")
{
$product = new JetPacks($crud, $productImages);
}
$productManager = new ProductManager($productTypes, $product);
return $productManager;
}
}You can of course extract the choice of product into a separate (private) function again ;)
then you'd use this as follows:
$anvil = AcmeFactory::create("anvil");
$anvil->create($weight);
$anvil->display($id);
...I am not that good with PHP, but I think you should somehow limit what you allow to be passed into your create function. In Java I'd solve this using an enum, but as I said, I don't know enough php :(
Apart from that, I have a minor nitpick too:
Be consistent in your use of whitespaces before parentheses. Compare:
$productImages = new ProductImages($crud);
$product = new Anvils ($crud, $productImages);Code Snippets
class AcmeFactory
{
public static function create($config)
{
$crud = new Crud;
$productTypes = new ProductTypes($crud);
$productImages = new ProductImages($crud);
$product = null;
if($config == "anvil")
{
$product = new Anvils($crud, $productImages);
}
else if($config == "jetpack")
{
$product = new JetPacks($crud, $productImages);
}
$productManager = new ProductManager($productTypes, $product);
return $productManager;
}
}$anvil = AcmeFactory::create("anvil");
$anvil->create($weight);
$anvil->display($id);
...$productImages = new ProductImages($crud);
$product = new Anvils ($crud, $productImages);Context
StackExchange Code Review Q#55030, answer score: 5
Revisions (0)
No revisions yet.