Friday, February 19, 2010

The Number one rule of design

The post on php's foreach construct raised a discussion about the design of the language and of userland code, so I'm recollecting my thoughts here. However, note that I was not discussing the utility of the construct, only the use of array functions when dealing with homogeneous transformation of an array, such as an array with exactly the same type or number of elements.

I would like to introduce a general principle I follow while designing an [object-oriented] system.
A design is good if it makes reacting to change easy.
Decoupling between different classes and packages can be measured by thinking about hypothetical changes to the requirements and how they can be accomodated. This is a double advantage: not only you will be more ready to embrace change, but you will be also writing testable code; unit testing is essentially a practice that change all the environment around a SUT by using stubs and mocks, to find out if the software unit complies to its contract.
Change will not always happen in the ways you have forethought, but high cohesion in software modules will help in dealing with it.
Anyway, I am not the person that invented this design rule (so you shouldn't call me a moron and go on with your practices.) It is contained in a famous paper titled On the Criteria To Be Used in Decomposing Systems into Modules, but we should call it the Number one rule of design:
We have tried to demonstrate by these examples that it is almost always incorrect to begin the decomposition of a system into modules on the basis of a flowchart. We propose instead that one begins with a list of difficult design decisions or design decisions which are likely to change. Each module is then designed to hide such a decision from the others.
This is really one of the most important rules in the information technology world. That's why you can plug in nearly everything in your computer in a standard interface, why you can drive any car and why a structure like Internet is possible. This is information hiding.

The discussion that started in the comments of the previous post is about how to pass an high number of parameters (and by high I mean more than 3) from a bunch of Client classes to a function or a method of a Server class. We will now apply the #1 rule of design to this problem.
The first solution is obvious: every parameter modelled as a formal parameter of the method. Though, a long signature lacks clarity, renders parameters order important and creates issues with default values.
A suggestion proposed in the comments is extending php syntax to automatically pass a map:
function foo($arg1, 'arg2' => 0, 'arg3' => false) {}
foo('arg1' => 42, 'arg2' => 10);
foo('arg1' => 42, 'arg2' => 10, 'arg3' => true);
which is syntactic sugar for:
function foo(array $args) {
    $args['arg2'] = isset($args['arg2'] ? $args['arg2'] : 0;
    ...
}
foo(array('arg1' => 42, 'arg2' => 10, 'arg3' => true));
and would make simpler passing maps as an argument.
My solution would be refactoring the method a bit and passing an Argument object as a parameter. An object will be particularly useful when the number of parameters is not fixed and there are many defaults.
Moreover, a separate object decouples default values handling by code execution. I think that passing an high number of parameters is often a smell since it's likely that a subset of these parameters will always be passed together, or one of them is repeatedly passed to different methods.
Let's consider passing normal arguments, passing a map (with or without sweet syntax, since it is functionally equivalent) versus using an Argument object.

Hypothetical change: adding a parameter.
Formal parameters: you have to modify the method signature and review the Client classes or add a default.
Map: you add a default for the parameter, if applicable.
Argument object: you can add the parameter to the Argument object if it fits; in other cases adding to the signature is a better choice.
Hypothetical change: deleting a parameter.
Formal parameters: you have to fix all the method calls.
Map: the method signature does not change.
Argument object: the method signature does not change.
Hypothetical change: adding another method with the same signature.
Formal parameters: duplicate and maintain the two signatures.
Map: duplicate the signature and maintain both the old and the new one, with the same default values.
Argument object: duplicate the one-parameter, short signature.
Hypothetical change: the default value for some parameters now has to be picked up from a configuration or a database.
Formal parameters: you have to add a collaborator in the Server class, and an argument to its constructor or a setter to inject it.
Map: you can tweak the definition but the signature remains the same. The collaborator problem is still present.
Argument object: create it with a factory that has a database-backed repository or a configuration parser as collaborator.
Hypothetycal change: a method parameter has to be validated, lazy calculated, and so on.
Formal parameters: no chance other than computing the value in the Clients. Maybe currying the method would work but it is a change of signature anyway.
Map: you have to compute the value internally, by adding collaborators and coupling, or externally, changing the Clients.
Argument object: the object or the factory that creates it effectively hides the parameter preprocessing or derivation, so no problem. Sometimes the Argument object itself can perform the calculation alone, or you can swap different subclasses of it. Endless possibilities.

Note that the higher the number of parameters, the more they are likely to change in computation, number and default value. That's why a method with 1 or 2 parameters is usually not an issue. An Argument object should comprehend parameters that change together. This object responsibility is decouple as much as possible the Server class from a [large] set of parameters.
Furthermore, an Argument object handles default values in relation to a default literal value with the power of a getter over a public property. You can hook pretty much everything in the creation and in the methods of an Argument object, protecting Clients and Servers from having to import the default argument from foreign packages, libraries and applications.
Finally, an Argument object captures a domain concept with a name, a protocol and a behavior, while a map is only a plain data structure. You can tell what a map contains, but you can't tell what a map does.
The code sample deals with calculation of a final price for an object, with three example of design for a Shop class.
<?php
class ShopWithFormalParameters
{
    /**
     * We use float for simplicity but a decimal structure would be
     * the correct choice.
     * @param float $regularPrice    price of the item
     * @param float $itemDiscount    discount on this item (percentual)
     * @param float $globalDiscount  discount of the shop during
     *                               sales period (percentual)
     * @param float $bonus           value of bonus coupons
     *                               used by the customer
     * @return float
     */
    public function getPrice($regularPrice,
                             $itemDiscount = 0,
                             $globalDiscount = 0,
                             $bonus = 0)
    {
        return $regularPrice
             * (1 - $itemDiscount / 100)
             * (1 - $globalDiscount / 100)
             - $bonus;
    }
}

$formalShop = new ShopWithFormalParameters();
printf("Price 1st item: %.2f\n", $formalShop->getPrice(200.00, 10, 20));
printf("Price 2nd item: %.2f\n", $formalShop->getPrice(200.00, 0, 20, 5.00));

class ShopWithMap
{
    /**
     * Syntax does not allow to pass a map easily, but
     * this signature is equivalent.
     * @param array $arguments  @see ShopWithFormalParameters::getPrice()
     *                          for keys
     * @return float
     */
    public function getPrice(array $args)
    {
        // similarly, the defaults would fit in the signature
        $args['itemDiscount'] = isset($args['itemDiscount']) ?
                                $args['itemDiscount'] : 0;
        $args['globalDiscount'] = isset($args['globalDiscount']) ?
                                  $args['globalDiscount'] : 0;
        $args['bonus'] = isset($args['bonus']) ? $args['bonus'] : 0;

        return $args['regularPrice']
             * (1 - $args['itemDiscount'] / 100)
             * (1 - $args['globalDiscount'] / 100)
             - $args['bonus'];
    }
}

$mapShop = new ShopWithMap();
printf("Price 1st item: %.2f\n", $mapShop->getPrice(array(
    'regularPrice' => 200.00,
    'itemDiscount' => 10,
    'globalDiscount' => 20
)));
// note that defaults had not to be specified if they
// precede actual parameters in the "signature"
printf("Price 2nd item: %.2f\n", $mapShop->getPrice(array(
    'regularPrice' => 200.00,
    'globalDiscount' => 20,
    'bonus' => 5.00
)));

// my take: refactoring towards a loosely coupled solution
class Item
{
    private $_regularPrice;
    private $_discount;

    /**
     * A couple of arguments always change together,
     * thus it makes sense to put them in the same argument object.
     * The item discount is totally encapsulated here,
     * so the new domain concept has been fairly useful
     * as now the ObjectOrientedShop does not even know a
     * item-specific discount exist.
     */
    public function __construct($regularPrice, $discount = 0)
    {
        $this->_regularPrice = $regularPrice;
        $this->_discount = $discount;
    }

    /**
     * @return float
     */
    public function getPrice()
    {
        return $this->_regularPrice * (1 - $this->_discount / 100);
    }
}

class ObjectOrientedShop
{
    private $_globalDiscount;

    /**
     * One argument changes rarely, or never.
     * So a setter or a constructor option can take care of it.
     */
    public function __construct($globalDiscount = 0)
    {
        $this->_globalDiscount = $globalDiscount;
    }

    /**
     * Only two arguments, which make introducing a map useless.
     * We are protected from changes that affect the Item:
     * adding an argument which is cohesive with Item should be
     * kept with the Item class. Global options should be kept in the
     * constructor or in a field reference anyway.
     * Per-call parameters such as bonus, if necessary, would be
     * wrapped with $bonus in a Sale object which can come from any
     * external service.
     * @param Item      we don't need docblocks to tell what Item is
     * @param float     bonus coupons value
     * @return float
     */
    public function getPrice(Item $item, $bonus = 0)
    {
        return $item->getPrice()
             * (1 - $this->_globalDiscount / 100) - $bonus;
    }
}

$ooShop = new ObjectOrientedShop(20);
printf("Price 1st item: %.2f\n", $ooShop->getPrice(new Item(200.00, 10)));
printf("Price 2nd item: %.2f\n", $ooShop->getPrice(new Item(200.00), 5.00));

No comments:

ShareThis