PHP Abstract Factory Pattern Implementation

Question

I am reading about the different patterns available out there. Currently I am on the Abstract Factory Pattern and I think I have a pretty good grasp on it. My resources are, besides wikipedia: http://www.tutorialspoint.com/design_pattern/abstract_factory_pattern.htm http://www.oodesign.com/abstract-factory-pattern.html https://github.com/domnikl/DesignPatternsPHP/tree/master/Creational/AbstractFactory

I am using Apple and its products to make a sample Abstract Factory pattern as an example. I understand that code duplication is bad design, hence the reason I am writing this. My code so far is:

abstract class AbstractAppleFactory {
    abstract public function createiPod( $capacity, $type, $color, $engraving );
    abstract public function createiPhone( $capacity, $type, $color, $antenna );
    abstract public function createComputer( $type, $HDCapacity, $CPU, $ram );
}

class iPodFactory extends AbstractAppleFactory {
    public function createiPod( $capacity, $type, $color, $engraving ) {
        $class = 'iPod' . $type;

        return new $class( $capacity, $color, $engraving);
    }

    public function createiPhone( $capacity, $type, $color, $antenna ){ /* no implementation necessary */}
    public function createComputer( $type, $HDCapacity, $CPU, $ram ){ /* no implementation necessary */}
}

interface iPlayer {
    public function play();
    public function stop();
    public function fastForward();
    public function rewind();
}

abstract class iPod implements iPlayer {
    protected $capacity;
    protected $color;
    protected $engraving;

    public function __construct( $capacity, $color, $engraving = null ) {
        $this->capacity = $capacity;
        $this->color = $color;
        $this->engraving = $engraving;
    }
}

class iPodClassic extends iPod {
    public function play() {/* implementation goes here */}
    public function stop() {/* implementation goes here */}
    public function fastForward() {/* implementation goes here */}
    public function rewind() {/* implementation goes here */}
}
class iPodShuffle extends iPod {
    public function play() {/* implementation goes here */}
    public function stop() {/* implementation goes here */}
    public function fastForward() {/* implementation goes here */}
    public function rewind() {/* implementation goes here */}
}

etc. etc. There is just too much code to put on here. I know that it is better to organize in directories and namespaces. That is not what I am learning right now. I am learning about patterns and OOP concepts.

The section in question is:

class iPodFactory extends AbstractAppleFactory {
    public function createiPod( $capacity, $type, $color, $engraving ) {
        $class = 'iPod' . $type;

        return new $class( $capacity, $color, $engraving);
    }

    public function createiPhone( $capacity, $type, $color, $antenna ){ /* no implementation necessary */}
    public function createComputer( $type, $HDCapacity, $CPU, $ram ){ /* no implementation necessary */}
}

Due to inheritance/abstraction I am forced to implement two unnecessary methods in an unrelated factory. createiPhone() and createComputer(). Am I doing the Abstract Factory Pattern right? Again, "code duplication is bad design!" What better way is there to go about this?

Answer 1

I think You've made a major mistake. Abstract factory purpose it to create an abstraction for creation of family of products, so you can easily change it to different family. f.e. Apple => Samsung :)

interface ComputerAbstractFactory {
    /**
    * @return Tablet
    */
    function createTablet();
    /**
    * @return Phone
    */
    function createPhone();
    /**
    * @return Computer
    */
    function createComputer();
}

class AppleComputerFactory implements ComputerAbstractFactory {}

class SamsungComputerFactory implements ComputerAbstractFactory {}

class IPad implements Tablet {}

class GalaxyTab implements Tablet {}

...

Using abstract factory pattern makes sense when You want to switch between both companies. Your code should depend only on abstractions (SOLID principles) ComputerAbstractFactory, Tablet, Phone, Computer. This way if you decide (f.e. by some configuration switch) which manufacturer should be used it's enought that you inject choosen implementation of ComputerAbstractFactory into your bussiness code and you are done - everything works, tablets get created, phones etc

You need to decide what abstraction you want to create in your application so the calling code will not be coupled to concrete implementations of that abstraction. I believe that IPlayer is what you want, where IPodClassic IPodShuffle are concrete implementations. If i'm right you should create IPlayerFactory

class IPlayerFactory {
    /**
    * @return IPlayer
    */
    function create() {...}
}

About the duplication you mentioned. If iPodFactory extends AbstractAppleFactory it should implement all it's method (btw if AbstractAppleFactory does not have any implementation it should be an interface) You broke Liskov principle from SOLID principles. Short version - if the calling code depends on AbstractAppleFactory and it gets iPodFactory as concrete implementation to operate on, it will break on calling createiPhone or createComputer - so abstraction does not work in that case.

Answer 2

You are on the right way, but i think you interface is wrong.

First of all, don't use an abstract class unless you need to create a method which will be used on the child classes. Use an interface will be better.

This would be my solution:

<?php
interface AppleFactoryInterface
{
    function create($data);
}

And here is an iPhoneFactory:

<?php
class iPhoneFactory implements AppleFactoryInterface
{
    public function create($data)
    {
        $klass = sprintf('iPhone'.$data['type']);
        $instance = new $klass;
        $instance->setFromArray($data);

        return $instance;
    }
}

As you can see, i have less code which is better to understand.