Skip to content

Latest commit

 

History

History
64 lines (52 loc) · 5.44 KB

5981.md

File metadata and controls

64 lines (52 loc) · 5.44 KB

Back to questions

Solution to 5981: Shapes

See code at solutions/code/tutorialquestions/question5981

An unforgivably bad design! In the baddesign1 package I show a very bad way to model this problem in Java. This design exploits the fact that both rectangles and ellipses are characterised by a pair of fields: width and height for a rectangle, semimajor and semiminor axes for an ellipse. The design uses a single class, Shape, to represent all shapes. The class has two int fields: first and second. For a rectangle, these represent width and height, respectively, while for an ellipse they represent the semimajor and semiminor axes. Hang on a minute? Maybe for a rectangle, first is height and second is width...I can't remember. And how would someone reading my code know, other than looking at some comment I may or may not have written? A Shape is equipped with a type code field, which is an enumeration that can be one of Ellipse, Circle, Square or Rectangle. Using type codes usually indicates poor OO design, especially if you find yourself testing the type code to decide how a method should behave. (This indicates that each type code should correspond to a different subclass, and that you should be overriding a (possibly abstract) method to provide different, separate, behaviours for each type.

A poor, but forgivable, design. As hinted in the question, it is not a good idea to have a class Circle that extends Ellipse. This is for three reasons:

  • If we create an Ellipse with equal semimajor and semiminor axes then we have actually created a circle, but the resulting object will not be an instance of Circle
  • If we create a non-circular Ellipse and then change one of its axes to make it circular, the object will still be an instance of Ellipse, but not Circle
  • Most drastically, if we create a Circle then, because a Circle is an Ellipse, we can then change e.g.~the object's semimajor axis to make it non-circular. We end up with an object that is an instance of Circle, but is not circular! We could try to overcome this problem by overriding setSemimajorAxis in Circle so that it correspondingly modifies the semiminor axis to keep the circular property. However, this would break what is known as the Liskov Substitution Principle: for an Ellipse we can prove that the axes can be changed independently; the LSP says that we should also be able to prove this of Circles, but our circularity-preserving hack would break this independence property

An analogous argument advises against making Square a subclass of Rectangle. A design with this flawed approach is presented in package baddesign2.

A good design. The preferred solution is have an abstract Shape class, extended by Ellipse and Rectangle, and not to have further Circle and Square subclasses. Instead, provide methods isCircle() and isSquare() that can be used to determine whether a shape is a circle or square. There are two choices as to where to place these methods. They could be abstract methods of the Shape that are overridden in both Rectangle and Ellipse: Rectangle can simply return false for isCircle, while Ellipse can test equality of semimajor and semiminor axes (similarly for isSquare with respect to Ellipse and Rectangle). This means that we can ask whether any shape is a circle or square, which is nice, but it does pollute the interface of Shape with details of specific kinds of shape. (Remember that the word interface is overloaded: here I am using it to refer to the public methods that Shape exposes, even though Shape is not a Java interface.) The alternative is to declare isCircle() in Ellipse and isSquare() in Rectangle. This avoids the pollution problem, but means that if we wish to find out whether an arbitrary shape is square, we must first find out whether it is a Rectnagle (using instanceof), and if so, cast it to Rectangle so that we can call isSquare().

As is often the case, there is no clear cut winner here. In the sample solution, I have used the former approach.

Another bad desgn. Package baddesign3 shows almost the sample solution, but the designer has been over-enthusiastic and tried to pull seemingly common fields of Ellipse and Rectangle into the abstract Shape superclass. Taking the "employ abstraction to remove duplication" principle too far, this designer has spotted that both Ellipse and Rectangle are represented by a pair of integers, and has decided to pull this commonality up into the Shape superclass. This is mistaken for three reasons:

  • The width of a rectangle represents something quite different to the semimajor axis of an ellipse. Thus although they can both be represented by an integer, they should not be represented by a common integer. The name widthOrSemimajorAxis smells of this problem.

  • If we decide to change the way we represent a rectangle then the coupled representation means that we need to consider the representation of an ellipse, even though this should be quite separate. The solution does not use encapsulation properly: rectangles and ellipses are not encapsulated from one another.

  • If we were to add additional shape classes, they probably would not share the feature of being represented by a pair of components (e.g., consider how you would represent a hexagon). Thus this solution is not extensible.