Simple Pizza Factory with tests

I implemented the Simple Factory Pattern with some unit tests, but I'm with a feeling that I did something wrong and that I can improve it.

Tell me what you think about the code (source and tests).

PizzaFactoryTest.php:

pizzaFactory = new PizzaFactory();
  }

  public function testPizzaFactoryShouldMakeAPizza()
  {
    $pizza = $this->pizzaFactory->make('greek');
    $this->assertInstanceOf('Pattern\SimpleFactory\Pizza', $pizza);
  }

  public function testPizzaFactoryShouldReturnNullWhenMakingANonexistentPizza()
  {
    $pizza = $this->pizzaFactory->make('nonexistent pizza');
    $this->assertNull($pizza);
  }

}

PizzaFactory.php:

 'Pattern\SimpleFactory\Pizza\Greek',
    'pepperoni' => 'Pattern\SimpleFactory\Pizza\Pepperoni',
  ];

  /**
   * @param string $name
   * @return null|Pizza
   */
  public function make($name)
  {
    if (isset($this->pizzas[$name]))
    {
      return new $this->pizzas[$name];
    }

    return null;
  }

}

PizzaStoreTest.php:

pizzaFactory = $this->getMock(PizzaFactory::class);
    $this->pizzaStore = new PizzaStore($this->pizzaFactory);
  }

  public function testPizzaStoreShouldReturnTheRequestedPizzaWhenOrdered()
  {
    $this->pizzaFactory->expects($this->once())->method('make')->with('greek');
    $this->pizzaStore->order('greek');
  }

}

PizzaStore.php:

factory = $factory;
  }

  /**
   * @param string $name
   * @return null|Pizza
   */
  public function order($name)
  {
    return $this->factory->make($name);
  }

}

PizzaTest.php:

```
pizza = $this->getMockForAbstractClass('Pattern\SimpleFactory\Pizza');
}

public function testPizzaShouldSetAndReturnTheExpectedName()
{
$pizzaName = 'Greek Pizza';
$this->pizza->setName($pizzaName);
$this->assertEquals($pizzaName, $this->pizza->getName());
}

public function testPizzaShouldSetAndReturnTheExpectedDescription()
{
$pizzaDescription = 'A Pepperoni-style pizza with dough, tomato, and cheese';
$this->pizza->setD

To me, this looks pretty much like a by-the-book example of the factory pattern. There are, however, a few areas that could be improved.

Missing Test Paths

I noticed that you have no test in PizzaFactoryTests which ensures that the produced Pizza is of the right type. As your tests states, you might be returning a Pizza, but not necessarily the one you would expect.

public function testPizzaFactoryShouldMakeAGreekPizza()
{
    $pizza = $this->pizzaFactory->make('greek');
    $this->assertInstanceOf('Pattern\SimpleFactory\Pizza\Greek', $pizza);
}

Encapsulation

I noticed that you use setters on the parent class to initialize your specialized classes. This works but breaks encapsulation. Instead, I would keep the setters private to the base class and expose a constructor that can take care of setting the properties. This also has the upside of enforcing that all pizzas are created equal. After all, in your system, is a pizza really a pizza when it has no name or description?

class Pepperoni extends Pizza {
    function __construct()
    {
        parent::__construct(
            'Pizza Pepperoni',
            'A Pepperoni-style pizza with dough, tomato, and cheese'
        );
    }
}

Mocking

You might want your factory to be able to return mocked objects when testing. This is useful to ensure that you are testing the factory in isolation, specially if the classes it instantiate are heavy to initialize. In that case, dependency injection is the way to go. There are many ways that you could accomplish this. In your case, you could inject the class path so that it points to a mocking pizza object.

class PizzaFactory {
    private $pizzas = [];

    function __construct($pizzas) {
        $this->$pizzas = $pizzas;
    }

    //...
}

// Usage
$properFactory = new PizzaFactory([
        'greek'     => 'Pattern\SimpleFactory\Pizza\Greek',
        'pepperoni' => 'Pattern\SimpleFactory\Pizza\Pepperoni',
]);

$mockedFactory = new PizzaFactory([
        'greek'     => 'Pattern\SimpleFactory\Pizza\Mock',
        'pepperoni' => 'Pattern\SimpleFactory\Pizza\Mock',
]);

This is a simple solution that will let you mock any path to the one you want, but it also leaks part of the internal logic. An alternative solution is to mock the instantiation operation entirely using an anonymous function.

class PizzaFactory {
    private $pizzas = [
        'greek'     => 'Pattern\SimpleFactory\Pizza\Greek',
        'pepperoni' => 'Pattern\SimpleFactory\Pizza\Pepperoni',
    ];

    private $maker = null;

    function __construct($maker) {
        $this->$maker = $maker;
    }

    public function make($name)
    {
        if (isset($this->pizzas[$name]))
        {
            return $maker($this->pizzas[$name]);
        }

        return null;
    }
}

// Usage
$properFactory = new PizzaFactory(function($className) {
    return new $className;
});

$mockedFactory = new PizzaFactory(function($className) {
    return new 'Pattern\SimpleFactory\Pizza\Mock';
});

There are probably other ways to mock the dependency on new but I am no aware of them. In the end, it all boils down to your requirement and how the PizzaFactory will evolve over time.

StackExchange Code Review Q#79262, answer score: 2