Does this code solve the square/rectangle Liskov Substution Principle example

design-patternsinheritanceliskov-substitutionobject-oriented-design

I just wanted to check that I understand the LSP correctly and can solve it. I am taking the classic rectangle/square problem and attempting a solution:

class Rectangle{
    public $width;
    public $height;

    function setWidth($width){
        $this->width = $width;
    }

    function setHeight($height){
        $this->height = $height;
    }
}

class Square extends Rectangle{

    function setWidth($width){
        $this->width = $width;
        $this->height = $width;
    }

    function setHeight($height){
        $this->height = $height;
        $this->width = $height;
    }
}

If you had some code like:

function changeSize(Rectangle $rect){
  $rect->setWidth(10);
  $rect->setHeight(30);
  $this->assertEquals(10,$rect->width);
  $this->assertEquals(30,$rect->height);
}

Then obviously rectangles and squares are not interchangeable, as square introduces a constraint to the parent class. Therefore, a square should not inherit from rectangles.

But surely we can agree that both square and rectangle are four sided shapes? This is my proposed solution, based on this premise:

abstract class AFourSidedShape{
    public $width;
    public $height;

    abstract public function __construct($width,$height);

    public function scaleUp($percentage){
        $this->height = $this->height + (($this->height / 100) * $percentage);
        $this->width = $this->width + (($this->width / 100) * $percentage);
    }

    public function scaleDown($percentage){
        $this->height = $this->height - (($this->height / 100) * $percentage);
        $this->width = $this->width - (($this->width / 100) * $percentage);
    }
}

class Rectangle extends AFourSidedShape{
    function __construct($width, $height){
        $this->width = $width;
        $this->height = $height;
    }
}

class Square extends AFourSidedShape{
    function __construct($width, $height){
        if($width != $height){
            throw new InvalidArgumentException('Sides must be equal');
        }else{
            $this->width = $width;
            $this->height = $height;
        }
    }
}

Our client code should be changed to something like:

function changeSize(AFourSidedShape $shape){
  $origWidth = $shape->width;
  $origHeight = $shape->height;
  $shape->scaleUp(10);
  $this->assertEquals($origWidth + (($origWidth/100) * 10),$shape->width);
  $this->assertEquals($origHeight + (($origHeight/100) * 10),$shape->height);
}

My theory is: rectangles and squares really are both foursidedshapes, so there shouldn't be a problem with inheriting from the foursidedshape abstract class. Whilst the square is still adding extra constraints in the constructor (i.e. throwing an error if the sides aren't equal), it shouldn't be a problem since we haven't implemented the constructor in the abstract parent class, and so client code shouldn't make assumptions about what you can/cannot pass into it anyway.

My question is: have I understood LSP, and does this new design solve the LSP problem for square/rectangle?

When using interfaces as suggested:

interface AFourSidedShape{
    public function setWidth($width);
    public function setHeight($height);
    public function getWidth();
    public function getHeight();
}

class Rectangle implements AFourSidedShape{
    private $width;
    private $height;

    public function __construct($width,$height){
        $this->width = $width;
        $this->height = $height;
    }

    public function setWidth($width){
        $this->width = $width;
    }

    public function setHeight($height){
        $this->height = $height;
    }

    //getwidth, getheight
}

class Square implements AFourSidedShape{
    private $width;
    private $height;

    public function __construct($sideLength){
        $this->width = $sideLength;
        $this->height = $sideLength;
    }

    public function setWidth($width){
        $this->width = $width;
        $this->height = $width;
    }

    public function setHeight($height){
        $this->height = $height;
        $this->width = $height;
    }

    //getwidth, getheight
}

Best Answer

I'm going to assume you are trying to solve the "Typical Violation" section of the LSP Wikipedia article. If that's the case, you haven't solved it and the section states clearly why. Specifically, start with the setup: "Square class that derives from a Rectangle class, assuming getter and setter methods exist for both width and height." LSP states that the subclass should be able to stand-in for the super class.

But, it's not just that! That's what you and your commenters are missing. If it was just a matter of torturing your design until you could make the swap (everything is a AFourSidedShape), you could simply design some generic object (no shape or behavior), have everything inherit from the object, and then swap in specific implementations. Think about what would happen if you did that. You'd constantly be interrogating the objects to determine what they can do and/or violating their post conditions That's what the LSP hates.

So in your case, a AFourSidedShape doesn't really solve anything because you'd constantly be checking if your specific implementations were Rectangles or Squares to ensure post conditions are correct -- a Square's width can magically change when you update its height but not a Rectangle's. That's what the sections means when it says, "these methods will weaken (violate) the postconditions for the Rectangle setters." You can't get away from less-than-ideal behavior. It's a tradeoff. (Unless you make your objects immutable! Hooray for immutability!)

Don't feel bad. The Rectangle/Square with getters/setters for width/height problem is meant to be unsolvable in the trivial sense. It's a great, simple example of why LSP is difficult in practice.

Related Topic