2
\$\begingroup\$

I'm making a road tax data calculator/parser for practice. I do this because I want to learn more about OOP and arrays with its functionality.

On my previous post, I made this same application but also included the ability to decode the data and putted it in right format. Now, I've already done that with a different application and got my data per vehicle type in a different PHP array in a different file.

A vehicle type array looks like this:

 return array (
  1 => 
  array (
    'benzine' => 16,
    'diesel' => 81,
    'lpg3_natural_gas' => 16,
    'lpg_others' => 92,
  ),
  551 => 
  array (
    'benzine' => 22,
    'diesel' => 99,
    'lpg3_natural_gas' => 22,
    'lpg_others' => 113,
  ),
  651 => 
  array (
    'benzine' => 29,
    'diesel' => 116,
    'lpg3_natural_gas' => 29,
    'lpg_others' => 134,
  ),
  751 => 
  array (
    'benzine' => 37,
    'diesel' => 137,
    'lpg3_natural_gas' => 37,
    'lpg_others' => 158,
  ),
  851 => 
  array (
    'benzine' => 50,
    'diesel' => 167,
    'lpg3_natural_gas' => 65,
    'lpg_others' => 182,
  ),
  951 => 
  array (
    'benzine' => 64,
    'diesel' => 193,
    'lpg3_natural_gas' => 93,
    'lpg_others' => 209,
  ),
  1051 => 
  array (
    'benzine' => 77,
    'diesel' => 219,
    'lpg3_natural_gas' => 121,
    'lpg_others' => 237,
  )
 );

Per each vehicle type, the format is different and it can rely on more or less property to calculate it's road tax prices.

The format on this vehicle type (Bus) is:

  • The vehicle's weight
  • The vehicle's fuel type

You can see that in the array above, the fuel types (benzine, diesel, etc) values are the prices belonging to that fuel type.


For every vehicle type, I made a class, some examples of those are:

  • PassengerCar
  • CampingCar
  • Bus
  • Motorcycle
  • MobileShop
  • DeliveryVan

All the vehicle type classes contain their needed properties to do the calculations, such as:

  • Its weight
  • Its fuel type

Some of those vehicle types have the same reliance on properties such as its weight and fuel type. Those are the PassengerCar and the CampingCar.

PassengerCar:

namespace Bas\VehicleRunningCostCalculator\Vehicle\Vehicles\Car\Cars;

/**
 * Use the "Car" class for polymorphism
 */
use Bas\VehicleRunningCostCalculator\Vehicle\Car\Car;


/**
 * Defines a passenger car, a passenger car needs the relies on the following data for calculating it's road tax
 * prices:
 *
 * - It's fuel type
 * - It's weight
 * - The passenger car's owner province
 *
 * @package   Bas\VehicleRunningCostCalculator\Vehicle\Vehicles\Cars
 *
 * @author    Bas van Driel <basvandriel94@gmail.com>
 * @copyright 2015 Bas van Driel
 * @license   MIT
 */
class PassengerCar extends Car
{

    /**
     * Instantiates a new passenger car class
     *
     * @param int   $fuelType The fuel type of the passenger car
     * @param float $weight   The weight of the passenger car
     */
    public function __construct($fuelType, $weight) {
        $this->fuelType = $fuelType;
        $this->weight   = $weight;
    }
}

CampingCar:

/**
 * Defines a camping car vehicle type, this vehicle type relies on the following data for calculating it's
 * road tax prices:
 *
 * - It's fuel type
 * - It's weight
 * - If it's rented or not
 * - The camping car's owner province
 *
 * @package   Bas\VehicleRunningCostCalculator\Vehicle\Vehicles\Cars
 *
 * @author    Bas van Driel <basvandriel94@gmail.com>
 * @copyright 2015 Bas van Driel
 * @license   MIT
 */
class CampingCar extends Car
{
    /**
     * @var bool $isRented If the camping car is rented or not
     */
    private $isRented;

    /**
     * Instantiates a new camping car class
     *
     * @param int   $fuelType The fuel type of the camping car
     * @param float $weight   The weight of the camping car
     * @param bool  $isRented
     */
    public function __construct($fuelType, $weight, $isRented) {
        $this->fuelType = $fuelType;
        $this->weight   = $weight;
        $this->isRented = $isRented;
    }

    /**
     * A getter for retrieving the $isRented variable in a safe way
     *
     * @return bool $isRented If the camping car is rented or not
     */
    public function isRented() {
        return $this->isRented;
    }
}

Because they have the same relies on properties, I've extended them to the Car abstract class:

/**
 * This class is a blue print for every "car" type, every car vehicle relies on it's fuel type and weight to
 * calculate the road tax prices for it
 *
 * @package   Bas\VehicleRunningCostCalculator\Vehicle
 *
 * @author    Bas van Driel <basvandriel94@gmail.com>
 * @copyright 2015 Bas van Driel
 * @license   MIT
 */
abstract class Car implements VehicleType
{
    /**
     * @var int $fuelType The fuel type of the car vehicle type
     */
    protected $fuelType;

    /**
     * @var float $weight The weight of the car vehicle type
     */
    protected $weight;

    /**
     * A getter for retrieving the $fuelType variable in a safe way.
     *
     * @return int $fuelType The fuel type of the car vehicle type
     */
    public function getFuelType() {
        return $this->fuelType;
    }

    /**
     * A getter for retrieving the $weight variable in a safe way.
     *
     * @return float $weight The weight of the car vehicle type
     */
    public function getWeight() {
        return $this->weight;
    }
}

That I've done with more vehicle types. I think I don't need to show that for this purpose.

The Car class extends the VehicleType class. This is just a simple interface to determinate if it's a VehicleType.

interface VehicleType
{

}

To parse the data belonging to the specific vehicle type, I use this class:

namespace Bas\VehicleRunningCostCalculator\DataParser;

/**
 * Use the "VehicleType" class for polymorphism
 */
use Bas\VehicleRunningCostCalculator\Vehicle\VehicleType;
use Bas\VehicleRunningCostCalculator\VehicleOwner\VehicleOwner;

/**
 * Use the "VehicleOwner" class for polymorphism
 */

/**
 * A class to retrieve the right data for the selected vehicle type based on the following (specific) user's
 * property's:
 *
 * - The vehicle type
 * - The vehicle's fuel type
 * - The vehicle's owner province
 * - If the vehicle owner is disabled
 *
 * @package   Bas\VehicleRunningCostCalculator\DataParser
 *
 * @author    Bas van Driel <basvandriel94@gmail.com>
 * @copyright 2015 Bas van Driel
 * @license   MIT
 */
class DataParserHandler
{
    /**
     * @var VehicleType $vehicleType The vehicle type the data has to be parsed for
     */
    private $vehicleType;

    /**
     * @var VehicleOwner $vehicleOwner The vehicle's owner
     */
    private $vehicleOwner;

    /**
     * Instantiates a new DataParserHandler class.
     *
     * @param VehicleType  $vehicleType  The vehicle type the data has to be parsed for
     * @param VehicleOwner $vehicleOwner The vehicle's owner
     */
    public function __construct(VehicleType $vehicleType, VehicleOwner $vehicleOwner) {
        $this->vehicleType  = $vehicleType;
        $this->vehicleOwner = $vehicleOwner;
    }

    /**
     * Retrieves the full qualified class names of every vehicle type data parser
     *
     * @return array The full qualified class names of every vehicle type data parser
     */
    public function resolveDataParsers() {
        $dataParsers = [];
        foreach (new \DirectoryIterator(__DIR__ . "\\DataParsers") as $dataParser) {
            if ($dataParser->isDot() || $dataParser->isDir()) {
                continue;
            }
            $dataParsers[] = "{$this->getNamespace()}\\DataParsers\\{$dataParser->getBasename(".php")}";
        }
        return $dataParsers;
    }

    /**
     * Resolves the instance of the right data parser belonging to the selected vehicle type
     *
     * @param array $dataParsers The full qualified class names of every vehicle type data parser
     *
     * @return DataParser The resolved data parser belonging to the user selected vehicle type
     */
    public function resolveDataParser(array $dataParsers) {
        //Get the class of the chosen vehicle type without it's namespace
        $vehicleTypeClass = substr(get_class($this->vehicleType), strrpos(get_class($this->vehicleType), "\\") + 1);

        //The fully qualified class name of the data parser belonging to this selected vehicle type
        $vehicleTypeDataParser = "{$this->getNamespace()}\\DataParsers\\{$vehicleTypeClass}DataParser";

        //Loop through all the found (resolved) data parsers and select the right one out of it
        for ($dataParsersIndex = 0; $dataParsersIndex < count($dataParsers); $dataParsersIndex++) {
            if ($dataParsers[$dataParsersIndex] == $vehicleTypeDataParser) {
                $dataParser = new \ReflectionClass($dataParsers[$dataParsersIndex]);
                return $dataParser->newInstance();
            }
        }
        return null;
    }

    /**
     * Retrieves the data from the resolved data parser belonging to the user selected vehicle type
     *
     * @param DataParser $dataParser The resolved data parser belonging to the chosen vehicle type
     *
     * @return array|int The resolved vehicle data belonging to the user's choices such as the vehicle type, fuel
     *                   type, where the vehicle owner is living
     */
    public function getData(DataParser $dataParser) {
        $data = $dataParser->resolveData($this->vehicleType, $this->vehicleOwner);
        return $dataParser->parse($data, $this->vehicleType, $this->vehicleOwner);
    }

    /**
     * @return string The namespace of this class
     */
    private function getNamespace() {
        return substr($this->getClass(), 0, strrpos($this->getClass(), "\\"));
    }

    /**
     * @return string The class name as string format
     */
    private function getClass() {
        return get_class($this);
    }
}

This loops through every DataParser class and selects out the right one based on the class name.

Here's one of the DataParser class:

class DeliveryVanDataParser implements DataParser
{

    /**
     * Resolves the right data based on the vehicle type in array format
     *
     * @param \Bas\VehicleRunningCostCalculator\Vehicle\VehicleType       $vehicleType
     * @param \Bas\VehicleRunningCostCalculator\VehicleOwner\VehicleOwner $vehicleOwner
     *
     * @return array The resolved data array for the selected vehicle type
     */
    public function resolveData(VehicleType $vehicleType, VehicleOwner $vehicleOwner) {
        /**
         * @type DeliveryVan $vehicleType
         */
        if ($vehicleOwner->isDisabled()) {
            return require "var/road-tax-data/DeliveryVanDisabledData.php";
        } elseif ($vehicleType->isCommercial()) {
            return require "var/road-tax-data/DeliveryVanCommercialData.php";
        }
        return require "var/road-tax-data/DeliveryVanPassengerData.php";
    }

    /**
     * @param array        $resolvedData The resolved data array for the selected vehicle type
     * @param VehicleType  $vehicleType  The selected vehicle type
     * @param VehicleOwner $vehicleOwner The vehicle owner belonging to the vehicle type
     *
     * @throws \Exception
     *
     * @return int
     */
    public function parse(array $resolvedData, VehicleType $vehicleType, VehicleOwner $vehicleOwner) {
        /**
         * @type DeliveryVan $vehicleType
         */
        $weight   = $vehicleType->getWeight();
        $fuelType = strtolower(FuelType::getFuelTypeName($vehicleType->getFuelType()));

        if ($vehicleOwner->isDisabled() || $vehicleType->isCommercial()) {
            return $data = $resolvedData[DataPropertyResolver::resolveWeightClass($resolvedData, $weight)];
        }
        $data = $resolvedData[DataPropertyResolver::resolveWeightClass($resolvedData, $weight)];
        if (!isset($data[$fuelType])) {
            throw new \Exception("Cant find the fuel type");
        }
        return $data = $data[$fuelType];
    }
}

Some of the utility methods such as resolveWeightClass are in this class:

class DataPropertyResolver
{
    /**
     * Resolves the "weight class" (not an actual class) based on the vehicle's weight and inputted data
     *
     * @param array $data          The inputted data array where the weight classes should exist
     * @param float $vehicleWeight The vehicle type's weight
     *
     * @return int The resolved weight class
     */
    public static function resolveWeightClass(array $data, $vehicleWeight) {
        $weightClasses = array_keys($data);
        for ($weightClassIndex = 0; $weightClassIndex < count($weightClasses); $weightClassIndex++) {
            $weightClass = $weightClasses[$weightClassIndex];

            //Define the next weight class in the array
            if ($weightClassIndex !== count($data) - 1) {
                $nextWeightClass = $weightClasses[$weightClassIndex + 1];
            } else {
                $nextWeightClass = $weightClass;
            }

            //The checking if the vehicle belongs to which weight class
            if (($vehicleWeight >= $weightClass) && ($vehicleWeight < $nextWeightClass)) {
                return $weightClass;
            }
        }
        return 0;
    }
}

This vehicle type (DeliveryVan) can rely on multiple properties to calculate its road tax data prices:

  • If the vehicle owner is disabled
  • If the delivery van is for commercial use
  • If the delivery van is for passenger use

Each of these properties have a different reliance on another set of properties such as:

  • Its weight
  • Its fuel type
  • etc

This extends the DataParser interface to determine if it's a "DataParser":

interface DataParser
{
    /**
     * Resolves the right data based on the vehicle type and vehicle owner's property's (not nessacery)
     *
     * @param VehicleType  $vehicleType  The selected vehicle type
     * @param VehicleOwner $vehicleOwner The vehicle owner belonging to the vehicle type
     *
     * @return array The resolved data array for the selected vehicle type
     */
    public function resolveData(VehicleType $vehicleType, VehicleOwner $vehicleOwner);

    /**
     * Parses the resolved data and returns the right data belonged on the vehicle type and vehicle owner's
     * property's
     *
     * @param array        $resolvedData The resolved data array for the selected vehicle type
     * @param VehicleType  $vehicleType  The selected vehicle type
     * @param VehicleOwner $vehicleOwner The vehicle owner belonging to the vehicle type
     *
     * @throws \Exception When it can't find the data in the resolved data array
     *
     * @return array|int The right data belonged on the vehicle type and vehicle owner's property's
     */
    public function parse(array $resolvedData, VehicleType $vehicleType, VehicleOwner $vehicleOwner);
}

To use this application, i use this code:

$vehicle = new \Bas\VehicleRunningCostCalculator\Vehicle\Vehicles\Van\Vans\DeliveryVan(FuelType::BENZINE,
                                                                                       800,
                                                                                       false);

$vehicleOwner = new \Bas\VehicleRunningCostCalculator\VehicleOwner\VehicleOwner($vehicle,
                                                                                Province::GELDERLAND, false);

$parser      = new \Bas\VehicleRunningCostCalculator\DataParser\DataParserHandler($vehicle, $vehicleOwner);
$dataParsers = $parser->resolveDataParsers();

$dataParser = $parser->resolveDataParser($dataParsers);
$data       = $parser->getData($dataParser);

var_dump($data);

This is just an example; this will be the vehicle choice of the user.

Problems

  • The code still looks messy (mostly in the DataParser classes).
  • The functionality isn't really split over multiple classes.
  • In the DataParser classes, I inject the VehicleType and VehicleOwner classes. Is this needed?
\$\endgroup\$
  • \$\begingroup\$ I don't have time to review all your code. But from skimming your DeliveryVanDataParser looks fishy. A data parser should only be parsing data, not including files. You should probably do that in a Controller class then inject the data you need to the parser. Then the parser can decide what to do with the data. Otherwise testing and substituting the class will be a PITA. \$\endgroup\$ – Tek Jan 22 '15 at 18:18
2
\$\begingroup\$

I couldn't help myself, so here are a few points.

Like I said in my comments.

  • class DeliveryVanDataParser

    • resolveData()

      Your DeliveryVanDataParser looks fishy, specifically resolveData. A data parser should only be parsing data, not including files. This should be the responsibility of another class such as a Controller. You should probably do that in a Controller class then inject the data you need to the parser. Then the parser can decide what to do with the data. Otherwise testing and substituting the class will be a PITA.

    • resolveDataParsers(), resolveDataParser()

      This is basically doing the job of a Factory class. The data parser shouldn't be instantiating or locating any class. Only parsing data. Instead the Factory that instantiates and locates classes should be injected to the parser from outside. Then the parser can fetch the objects it needs.

Injecting Vehicle Type and Vehicle Owner

If the class needs it in order to do it's job, why not?

However I'd rename DataParserHandler to AbstractDataParser along with an interface to implement the methods all parser types will use. Such as parse() method.

then create a child class class VehicleParser extends AbstractDataParser and class VehicleOwnerParser extends AbstractDataParser. Anything specific to parsing VehiclesType and VehicleOwner will be handled by these classes. Then you can combine their information if you need them elsewhere via another class. Or you could have Vehicle inside VehicleOwner too. Just depends on your needs.

However, the outlined tips above don't all apply to your code.

This is a suggestion on what I'd refactor the code to:

//Controller->vehicleDataPage();

$vehicleType = new DeliveryVan();
$vehicleOwner = new Owner();

$rawData = ...; //Doesn't matter what or where your data is, just fetch it.

$VehicleParser = new VehicleParser();
$VehicleOwnerParser = new VehicleOwnerParser();

//You can populate $Vehicle's properties inside parse() through $Vehicle setters.
$VehicleParser->parse($rawData, $Vehicle);

$Vehicle->resolveWeightClass();

//Do the same with owner
$VehicleOwnerParser->parse($rawData, $Owner);

//$Vehicle and $Owner should be populated by now.
//You can do with the data as you wish in an OOP matter.

Just to explain, some of the methods could be simplified

Since objects are passed by reference you could populate $Vehicle from inside $VehicleParser->parse(). $Vehicle would have all it's fields populated by just calling parse function alone.

Then once the properties inside $Vehicle are populated via it's setters you could move DataPropertyResolver::resolveWeightClass() to $Vehicle->resolveWeightClass() and it could just do it itself since it involves it's own properties.

Bonus tip

Avoid using static methods. You may use them as long as they don't call other static methods inside. Here's some more information on this.

In general, I'd just stay away completely from static methods so the dependencies of an object are always clear to other developers.

Yet another good read: http://kore-nordmann.de/blog/0103_static_considered_harmful.html

Taken from: https://stackoverflow.com/a/5961194/174368

Those are my suggestions for now.

\$\endgroup\$
  • \$\begingroup\$ Isn't DataResolver a beter name then Controller then? Because it will actually get data (arrays) from that specific file. \$\endgroup\$ – Bas Jan 22 '15 at 20:22
  • \$\begingroup\$ @Bas Controller is a general term that takes care of wiring your all code together. These are the calls that the Controller makes before rendering a webpage, or API, etc since the Controller also takes care of calling the class to present your data to an HTTP response. Basically each class method in a Controller class is a web page. Everything to present that web page is prepared inside that method. \$\endgroup\$ – Tek Jan 22 '15 at 20:26
  • \$\begingroup\$ However, you can ignore the Controller part, it's common to present web pages with controllers in modern applications so I only used the term. \$\endgroup\$ – Tek Jan 22 '15 at 20:33
  • \$\begingroup\$ but if i wanted to do it, there should be a Controller for every vehicle type? Also, there isn't an VehicleOwnerParser :) \$\endgroup\$ – Bas Jan 22 '15 at 20:34
  • \$\begingroup\$ @Bas Imagine Controller functions like web pages. Each function returns a different page. So in $Controller->vehicleDataPage(); inside you call everything needed to render that page, that's all. \$\endgroup\$ – Tek Jan 22 '15 at 20:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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