6
\$\begingroup\$
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);
.....
\$\endgroup\$
1
  • \$\begingroup\$ why the new keyword? You can just do AcmeFactory::anvil(); \$\endgroup\$ Commented Jun 23, 2014 at 15:30

1 Answer 1

5
\$\begingroup\$

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.

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);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.