3
\$\begingroup\$

Introduction:

I have a website where customers can configure and checkout glass products like for example: Shower doors, Mirrors, Kitchen splashbacks, Table tops etc.

What I am trying to do:

I am trying to create a class for calculating the price of this kind of configurable products. All the products have in common that they use square meters to calculate the price and some of the products have the same way of calculating the glasstype price.

The idea:

I have created an abstract class with the methods and properties which the products have in common. The idea is to create a class for each product with their exceptions (own methods and properties).

My question:

Am I doing the right thing or should I approach this differently? For example calling the methods in the constructor in my showerdoor class.

The abstract class:

abstract class ConfigurableProduct {

    public $squareMeters;

    // Stores the prices
    protected $_price = array();

    // Initializes object
    public function __construct( $data = array() ) {

        // Set square meters
        $this->squareMeters = $data['square_meters'];

    }

    // Adds price to _price array   
    public function addPrice( $value ) {
        $this->_price[] = $value;
    }

    // Sums prices in _price array
    public function getPrice() {
        return array_sum( $this->_price );  
    }

}

Showerdoor class:

class Showerdoor extends ConfigurableProduct {

    // Initializes object
    public function __construct( $data ) {

        // Parent constructor
        parent::__construct( $data );

        // Set glasstype
        if ( isset( $data['glasstype'] ) )
        $this->setGlasstype( $data['glasstype'] );

        // Set coating
        if ( isset( $data['coating'] ) )
        $this->setCoating( $data['coating'] );

        // Set hinges
        if ( isset( $data['hinges'] ) )
        $this->setHinges( $data['hinges'] );

    }

    // Adds the glasstype price to the _price array
    public function setGlasstype( $id ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price
            $value = gtp_wizard_part_price( $id ) * $this->squareMeters;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

    // Adds the coating price to the _price array
    public function setCoating( $id ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price
            $value = gtp_wizard_part_price( $id ) * $this->squareMeters;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

    // Adds the hinges price to the _price array
    public function setHinges( $id, $quantity = 2 ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price (multiply for 2 hinges)
            $value = gtp_wizard_part_price( $id ) * $quantity;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

}

How I create the object:

// Dimensions
$width          = 800;
$height             = 2000;
$square_meters  = $width * $height / 1000000;

// Parts (ids from database)
$glasstype      = 282236;
$hinges         = 282242;
$coating        = 282296;

// Create object
$showerdoor = new Showerdoor(
     array( 
        'square_meters'     => $square_meters,
        'glasstype'         => $glasstype, 
        'hinges'            => $hinges,
        'coating'       => $coating,
    ) 
);
// Echo price
echo $showerdoor->getPrice();
\$\endgroup\$
1
\$\begingroup\$

General concept

From your explanation it seems to me that, apart from dimensions and prices, different categories of products may have different additional features (like glasstype, hinges, and coating in your showerdoor example).

But since you showed only one product, I can't be certain. So What follows is based on the hypothesis that:

  • there is potentially a number of these features and each category of product uses only some of them, but each feature may be shared by several categories.
  • for a given feature, the way it is processed is the same for all involved categories of products.

That's said, I think you'd better to write all these methods in your abstract class, so they're always already available for each new derived class you create for a kind of product.

More, I think that even if certain features are specific to certain unique categories (what you called "exceptions"), they also should be written in the abstract class.

At the same time, in order to keep the whole process secure (i.e. be sure that a derived class can't accept features that don't regard their category), each derived class could have its own allowed-features list.

Applying the above strategy leads to something like this for the abstract class:

abstract class ConfigurableProduct {

  public $squareMeters;
  // Stores the prices
  protected $_price = array();

  // Initializes object
  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['square_meters'];

    // Set features when they are both allowed and present
    foreach ($allowed_features as $feature) {
      if (isset($data[$feature])) {
        $method = 'set' . $feature;
        $this->$method($data($feature));
      }
    }
  }

  // Adds price to _price array   
  public function addPrice( $value ) {
    $this->_price[] = $value;
  }

  // Sums prices in _price array
  public function getPrice() {
    return array_sum( $this->_price );  
  }

  // One example of feature method for "someFeature"
  public function setSomeFeature($id) {
    // Check for part id (so integer)
    if (isset($id) && is_int($id)) {
      // Calculate price
      $value = gtp_wizard_part_price($id) * $this->squareMeters;
      // Add price to price array
      $this->addPrice( $value );
    }
  }

  // Other feature methods here...
}

And something like this for a category of product:

class Showerdoor extends ConfigurableProduct {

  protected $allowed_features = [
    'glasstype',
    'coating',
    'hinges',
  ];

  // Initializes object
  public function __construct($data) {
    // Parent constructor
    parent::__construct($data, $this->allowed_features);
  }
}

At my eyes this way ensures both consistency and simplicity (hence readability): all existing feature methods are gathered at a unique place, once for all; granting a feature to a category is only matter of simple registration.

Some minor aspects

I'm a bit surprised you calculate square meters outside of the object: unless I'm missing something subtle, it could be done (again, once for all) in the abstract class, so the __construct() function becomes:

  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['width'] * $data['height'] / 1000000;
    // ...

And the object creation is only:

// Parts (ids from database)
$glasstype      = 282236;
$hinges         = 282242;
$coating        = 282296;

// Create object
$showerdoor = new Showerdoor(
   array( 
    'width'     => 800,
    'height'    => 2000,
    'glasstype' => $glasstype, 
    'hinges'    => $hinges,
    'coating'   => $coating,
  ) 
);

Also I'd suggest adding the availibility for the app to access data in the object, avoiding to keep global variables outside of it.
This way, the abstract class looks like this:

abstract class ConfigurableProduct {

  public $squareMeters;
  public $data;
  // Stores the prices
  protected $_price = array();

  // Initializes object
  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['width'] * $data['height'] / 1000000;
    // Keep a copy of data
    $this->data = $data;
    //...

And the object creation is even simpler:

// Create object
$showerdoor = new Showerdoor(
   array( 
    'width'     => 800,
    'height'    => 2000,
    'glasstype' => 282236, 
    'hinges'    => 282236,
    'coating'   => 282296,
  ) 
);

Regarding the feature methods you gave in example, note that setGlasstype() and setCoating() are perfectly identical and hence can be factorized.
Obviously it can be achieve by simply invoking a common (protected) method from each of them. But if this situation is frequent you might use another strategy, using aliases, like this:

  // Aliases for feature methods
  protected $feature_aliases = [
    'glasstype'   => 'GlassType_setCoating',
    'setcoating'  => 'GlassType_setCoating',
    'sethinges'   => 'SetHinges',
  ];

So the __construct() function must be adapted:

// Set features when they are both allowed and present
foreach ($allowed_features as $feature) {
  if (isset($data[$feature])) {
    $method = 'set' . $this->feature_aliases[$feature];
    $this->$method($data($feature));
  }
}

This way, again, we get more consistency and readability, since there is a clear and brief registry for all existing feature methods.
Note that this also simply solves an issue I didn't pay attention in the above: the difference between all-lower and camelcase feature names.

Hope this helps!

\$\endgroup\$
  • \$\begingroup\$ Thank you for your very helpful answer and you time to write it! The reason why I chose to calculate the square meters outside of the class is because I earlier learnt that this kind of calculations are a more general thing so not place it inside this kind of specific classes. Because maybe you have another class or function that needs this calculation too, and then you write more code than needed. What are your thoughts about this? \$\endgroup\$ – Robbert Jan 13 '16 at 9:44
  • \$\begingroup\$ But otherwise, maybe in the future I need to extend this class and need only the height for example and then I only have the square meters.. \$\endgroup\$ – Robbert Jan 13 '16 at 9:45
  • \$\begingroup\$ @Robbert Since you're building a real app, there are many conceptual aspects that can't be considered without a large vision of all the constraints, while here I only focused on the limited part you exposed. So sure: with a larger vision you can (and even must) take all aspects in account and make things differently than what I suggested. E.g.: in the case you evoked about calculations, you might choose to have another, separated, class with (maybe only) static methods such as calcSquareMeters(), called from ConfigurableProduct and also from other parts of your app. \$\endgroup\$ – cFreed Jan 13 '16 at 15:05

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.