Does this code solve the square/rectangle Liskov Substitution Principle example?

Question

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
}

Answer 1

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.

Answer 2

The LSP is about the contract of a class, and that inherited classes must still fulfill the same contract as their base class. An interface just in code will typically only define parts of that contract, mainly the syntactical part, and the semantics might be partially given by descriptive names of the methods or the parameters. Other parts of the contract are often just defined in comments, by adding "assertion statements", by making use of specific language features, or they might be encoded into unit tests.

Thus if you solved the LSP violation really, depends on the full semantic contract of your interface. If the contract looks like this (which is IMHO the more obvious behaviour):

// contract: a four sided shape is an object with two individual, independent
// properties "width" and "height"
interface AFourSidedShape{
    public function setWidth($width);
    public function setHeight($height);
    public function getWidth();
    public function getHeight();
}

then Square class does not fulfill the same contract as its base class, so it still violates the LSP. One could write that more formally in terms of "post conditions", checking that each time you call setHeight, the value of getWidth does not change, and vice versa.

If, however, your semantic contract looks like this:

 // contract: a four sided shape is an object with two 
 // properties "width" and "height" which must not 
 // be assumed to be independent; maybe changing one can change the other
 interface AFourSidedShape{
     // ...
 }

then there is no LSP violation any more. From your question and the way you describe the constructor constraints, I guess that is the contract you have in mind. However, the latter might violate the principle of least astonishment, a "setter" which for some objects changes another value, and for others it does not, can be mind-boggling to the average user of your class. Better avoid to put such setters in the common interface, LSP obeyed or not.

Side note: of course, the original Square/Rectangle problem can be understood as "is there an implementation where Square derives from Rectangle or vice versa directly which does not violate the LSP". If one reads it this way, then @ScantRoger is correct and your design solves a different problem. However, I read the problem quite differently as is there a way to incorporate inheritance to deal with rectangles and squares in a generic manner without violating LSP, and for this problem the answer is "yes, your suggestion will solve that problem".

Answer 3

In a practical way, that has been solved many times.

Programs like Inkscape, Photoshop, The Gimp, Illustrator and so on don't have separate rectangle and square tools in their palettes. They have only one tool for drawing rectangles and squares. And they don't call it "quadrilateral", they call it rectangle. Some even call it "rectangles and squares tool" so confused users wouldn't migrate to other apps that would allow them to draw squares.

So it seems to me that they found a way for it to work for them. And I suspect what they have done is assume a square is just a rectangle with equal height and width. They even allow you, by using the shift key when drawing, to lock it in a way that you can create a square.

I don't really think those programs switch from a rectangle object to a completely different and incompatible square object when you press shift, only to fall-back to an incompatible rectangle when you release Shift. That could be easily corroborated by downloading the source code of those programs that are open-source, like Inkscape or The Gimp.

So I think that when you changed the design from inheritance of a superclass to implementation of an interface, you "kinda" solved it.

Because in the case of inheritance you are stating that a square is a rectangle, which is false. In the other hand when you go with implementing an interface you are saying that a square can act as a rectangle. In that scenario what the methods do or don't do under the hood is an implementation detail. Better yet if you forget about squares altogether you are fine because nothing prevents you from creating a rectangle with equal sides.

So it's really a philosophical problem and not a practical one.

In Wikipedia definition

A square...It can also be defined as a rectangle in which two adjacent sides have equal length.

Answer 4

With the LSP and inheritance basically means: the child class can do everything that the parent class can and a bit more.

And here comes the problem:

• For a square : you can fold a square into a half-square triangle. Which mean you could make a method: "makesquaretriangle". This is not true for all rectangles.

• For a rectangle : If you multiply the height by a factor, and the width by the inverse of that factor, it will still have the same surface area. Which means you could make a method: "doublemyheightbutkeepsurface" which doubles the height and automatically halves the width.
  This is not true for a square.

So you see, both square and rectangle have methods that don't make sense on the other, that's why they can't always be substituted. And why they can't inherit from each other either way.

The LSP is specifically for the cases where you want to either inherit A from B or want to inherit B from A, but can't. Other cases where you inherit both B and A from a common base class are out of the scope of the LSP. So your solution has nothing to do with the LSP.

Answer 5

I see that is an old question but I want to give my 2 cents.

You didn't slove the problem and your design doesn't respect LSP, because one user could have this code :

AFourSidedShape square = new Square(5, 5);
... //Later in code.
shape.setWidth(4);
shape.setHeight(3);
... //And later.
shape.getHeight() * shape.getWidth() // The user would expect its area is 12 but its currently 9.

First: try to manipulate shapes in the most general way as you can, squares are rectangles, so manipulate squares as rectangles and let other code handle the rest, for example, if you have a paint program it would be good to let users create squares, rectangles when a user creates a squares using the GUI, in reality, it creates a rectangle which has the same width and height and then let the UI shows it, for resize operations, you could also offer the ability to grow and maintain the width/height by a percentage, the controller of this action would apply the general formula and it will works.

Second: You should trust in objects, that's what encapsulation is, Try to put common properties on the interface for example :

interface Shape { //This interface represents all the 2D shapes.
    double getArea();
    double getPerimeter();
}

class Rectangle implements Shape {
    private width;
    private height;
    ...
    public double getArea() { return width * height; }
    public double getPerimeter() { return width * 2 + height * 2; }
}

and then implements that methods on the specified classes, by doing that, you are encapsulating the area and perimeter calculation inside the class, this is definitely better than calculating these properties outside the shape object, because it reveals that you trust in what object will give you, but when you do it outside the object, it seems as you don't trust in that object and you are assigning this responsibility to another object (which in fact you trust in it).

Answer 6

AFoursidedShape hides information about the dependencies between height and width, so LSP is not violated as is the case with Square inhereting from Rectangle.

The class rectangle and square are representations of squares and rectangles, therefor they do not necessary share the same relationship as what they represent. In other words a mathemtical Square has a relationship with mathemtical rectangle, but that does not imply the class square must necesseraly have same relationship with class rectangle. If we still want to treat Square as a Rectangle another way of doing it without violating LSP could be to have an interface thats called ARectangleReader which only has contract for the getters getHeight and getWidth which is implemented by the Square class and Rectangle class.

Another way to think about it: does a mutable Square really behave like a mutable Rectangle?

Answer 7

You are completely missing the point. If I change the width of a rectangle then the height is unchanged. Your square class breaks that.

As long as you are aware of the LSP violation, feel free to make square a subclass of rect. Change the implementation of setWidth and setHeight to assert(). If you like add a method setWidthAndHeight() which asserts if the result is not a square. Use what you can from the base class.