dimanche 6 novembre 2016

Square Rectangle implementation that does not break SOLID / LSP

There are several threads on SO pertaining to the classic example of design that breaks the Liskov Substitution Principle: the Square and Rectangle classes. The problem starts by demonstrating that if Square extends Rectangle it violates LSP. Many people ask how to solve this. I've heard ideas including having a Polygon base class that is extended by Rectangle and Square etc. I thought about this too and to me this is the wrong direction - the wrong discussion. Square does not extend Rectangle. A square does not add capability or properties to a rectangle. A square is a rectangle. It's a type of rectangle.

Given that a Rectangle class exists and i'm tasked with building a Square class reusing Rectangle I would build a Square class with a protected property that is an instance of Rectangle. I would expose all properties/methods of Rectangle that also apply to Square and not expose those that do not. Any properties or methods of Square that do not apply to Rectangle would appear in my Square class. Here's my solution in PHP:

class Square {

    protected $rectangle;   // instance of Rectangle

    function __construct ($length) {
        if (!is_numeric ($length))
            throw new Exception ("Square::__contruct - length must be numeric");
        $this->rectangle = new Rectangle ($length, $length);  // given Rectangle (width, height)
    }

    public function getArea () {
        return $this->rectangle->getArea();
    }

    public function getLength () {
        return $this->rectangle->getWidth();  // or height pick one
    }

    public function setLength ($length) {
        if (!is_numeric ($length))
            throw new Exception ("Square::setLength - length must be numeric");
        $this->rectangle->setWidth ($length);
        $this->rectangle->setHeight ($length);
    }

    public function isSquare() {
        return ($this->rectangle->getHeight() == $this->rectangle->getWidth());
    }

}

As the argument goes the reason why breaking LSP in the case of Square extends Rectangle is problematic is that someone else adds a method to Rectangle that alters the area of the Rectangle by some factor - let's call that a method with a signature like

public function transformArea (factor) 

that is implemented as multiplying the width by factor. The argument states that now your squares will grow by the square of the factor because your width is the same as the height and if the width grows by 10% the square grows by 21%. This is not exactly true because this is not how these system actually work. What actually happens is that the width grows by factor and the area of the rectangle also grows by factor but now instances of Square would no longer actually be squares (in the case of Square extends Rectangle) since width and height are no longer the same. In my version someone else can add that method to Rectangle and it does not break Square - nobody can use the transformArea method of the Rectangle on instances of Square because it has not been exposed by the Square class yet.

If someone dictates that this method be added and then another programmer adds it to the Square class by simply exposing it from the Rectangle class like such:

public function transformArea ($factor) {
    $this->rectangle->transformArea ($factor);
}

then assuming this person does their job and runs a unit test on the class they will discover that they just broke isSquare whenever running this method. But that test may get added to the end of a stack of tests and may not reveal the bug. But that's a problem with development in general not necessarily a design flaw in this system.

The correct answer is to implement the method in Square by increasing length by the square root of factor. Like this:

public function transformArea ($factor) {
    if (!is_numeric ($factor))
        throw new Exception ("Square::transformArea - factor must be numeric");
    if ($factor <= 0)
        throw new Exception ("Square::transformArea - factor must be greater than zero");
    $growBy = (float) sqrt ($factor);
    $newLength = $this->getLength() * $growBy;
    $this->setLength ($newLength);
}

Another argument I can imagine is that this class does not guarantee the Rectangle inside it remains a square (width == height). I disagree. Square does not expose methods that change either and not both. Rectangle has methods that could break that but none are exposed.

Personally this fits the real world more to me. A Square is a Rectangle. All Squares are Rectangles but not all Rectangles are Squares.

Does my solution break any rules of SOLID?

Aucun commentaire:

Enregistrer un commentaire