Car store implementation using abstract factory

Question

I'm studying design patterns and I made this car store implementation. Although I'm confident that the idea is right, I'm not confident that the implementation is good. I would like you to check it out and give me some advice.

It doesn't have tests, and I'm using an abstract class with a private method.

Car:

<?php namespace Pattern\AbstractFactory;

interface Car {

}

CarFactory:

<?php namespace Pattern\AbstractFactory;

use InvalidArgumentException;

abstract class CarFactory {

  private $sports = [];
  private $family = [];

  function __construct(array $sports, array $family)
  {
    $this->sports = $sports;
    $this->family = $family;
  }

  public function makeSportsCar($name)
  {
    return $this->makeCar($this->sports, $name);
  }

  public function makeFamilyCar($name)
  {
    return $this->makeCar($this->family, $name);
  }

  private function makeCar(array $cars, $name)
  {
    if (! isset($cars[$name]))
    {
      throw new InvalidArgumentException('You tried to make a nonexistent car');
    }

    return new $cars[$name];
  }
}

CarStore:

<?php namespace Pattern\AbstractFactory;

class CarStore {

  private $factory;

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

  public function orderSportsCar($name)
  {
    return $this->factory->makeSportsCar($name);
  }

  public function orderFamilyCar($name)
  {
    return $this->factory->makeFamilyCar($name);
  }

}

Chevrolet:

<?php namespace Pattern\AbstractFactory\Factory;

use Pattern\AbstractFactory\CarFactory;

/**
 * @codeCoverageIgnore
 */
class Chevrolet extends CarFactory {

  private $sports = [
    'miray' => 'Pattern\AbstractFactory\Car\Sport\Miray'
  ];

  private $family = [
    'equinox' => 'Pattern\AbstractFactory\Car\Family\Equinox'
  ];

  function __construct()
  {
    parent::__construct($this->sports, $this->family);
  }

}

Equinox:

<?php namespace Pattern\AbstractFactory\Car\Family;

use Pattern\AbstractFactory\Car\Family;

class Equinox implements Family {

}

Evos:

<?php namespace Pattern\AbstractFactory\Car\Sport;

use Pattern\AbstractFactory\Car\Sport;

class Evos implements Sport {

}

Family:

<?php namespace Pattern\AbstractFactory\Car;

use Pattern\AbstractFactory\Car;

interface Family extends Car {

}

Ford:

<?php namespace Pattern\AbstractFactory\Factory;

use Pattern\AbstractFactory\CarFactory;

/**
 * @codeCoverageIgnore
 */
class Ford extends CarFactory {

  private $sports = [
    'evos' => 'Pattern\AbstractFactory\Car\Sport\Evos'
  ];

  private $family = [
    'galaxy' => 'Pattern\AbstractFactory\Car\Family\Galaxy'
  ];

  function __construct()
  {
    parent::__construct($this->sports, $this->family);
  }

}

Galaxy:

<?php namespace Pattern\AbstractFactory\Car\Family;

use Pattern\AbstractFactory\Car\Family;

class Galaxy implements Family {

}

Miray:

<?php namespace Pattern\AbstractFactory\Car\Sport;

use Pattern\AbstractFactory\Car\Sport;

class Miray implements Sport {

}

Sport:

<?php namespace Pattern\AbstractFactory\Car;

use Pattern\AbstractFactory\Car;

interface Sport extends Car {

}

Answer 1

use protected instead of private

by making $sports and $family protected you could just override them within the child classes without having to use parrent::__construct.

interface vs abstract

why Interface for Car? It is not wrong or something but since you are asking for feedback i would recommend to use an abstract class for Car and extend it for implementations of car like Galaxy. You should ask yourself "what other object than specific cars would implement the interface "car" afaik no one.

But if you would have an Interface like ABS you could ask questions like "does instance of car implement ABS" which sounds more like the real world you are trying to model

separation of sport && family cars

i dont know why you separate them like this, it looks like you could simplify the code a lot without this kind of separation.

my simplified take:

    <?php
namespace Car {

    abstract class Car
    {
    }

    class Shop
    {
        private $factory;

        public function __construct(\Car\Factory\Factory $factory)
        {
            $this->factory = $factory;
        }

        public function orderCar($name)
        {
            return $this->factory->makeCar($name);
        }
    }



abstract class Ford extends Car {}
class Mustang extends Ford {}

}
namespace Car\Factory{
abstract class Factory {

    protected $cars = array();
    abstract public function makeCar($name);
}

class Ford extends \Car\Factory\Factory {

    protected $cars = array("Mustang" => '\Car\Mustang');
    public function makeCar($name) {
        if (!isset($this->cars[$name])) {
            throw new \InvalidArgumentException(sprintf("%s %s \n", "invalid car name", $name));
        }

        return new $this->cars[$name];
    }

}
}
namespace App {

use Car\Factory\Ford as FordFactory;
use Car\Shop;

try {
    $fac = new FordFactory();
    $shop = new Shop($fac);
    $car = $shop->orderCar('Mustang');
    var_dump($car);

    if ($car instanceof \Car\Ford) {
        echo "real deal \n";
    }
} catch (\Exception $e) {
    print_r($e->getMessage());
}
}