André Restivo

SOLID

1. Single Responsibility (SRP)

public class AreaAggregator {
  private List<Shape> shapes = new ArrayList<>();

  public void addShape(Shape shape) {
    shapes.add(shape);
  }

  public double sum() {
    double sum = 0;
    for (Shape shape: shapes) {
      if(shape.getClass().equals(Circle.class)) {
        sum += Math.PI * Math.pow(((Circle) shape).getRadius(), 2);
      } else if (shape.getClass().equals(Square.class)) {
        sum += Math.pow(((Square) shape).getSide(), 2);
      }
    }
    return sum;
  }

  public String output() {
      return "Sum of areas: " + sum();
  }
}

Identifying the Issue

We now have an elementary but working program. Let's now add two more features:

<area>25</area>

Fixing the Issue

//...
AreaStringOutputter stringOutputter = new AreaStringOutputter(aggregator);
AreaXMLOutputter xmlOutputter = new AreaXMLOutputter(aggregator);

System.out.println(stringOutputter.output());
System.out.println(xmlOutputter.output());

2. Open/Closed (OCP)

Add a new shape: Rectangle. A Rectangle has a width and height.

Identifying the Issue

"A module should be open for extension but closed for modification."

Fixing the Issue

Move the area calculation to the Shape class and implementations by creating a getArea() method in all of them. The getArea() method in the Shape class and the Shape class itself should now be abstract, like this:

public abstract class Shape {
  public abstract double getArea();
}

A better solution would be to have Shape as an interface and all subclasses implementing this interface instead of extending a class:

public interface Shape {
  double getArea();
}

Lets do it this way, and then:

3. Liskov Substitution (LSP)

Identifying the Issue

Realize that this violates the LSP (Liskov Substitution Principle) as you cannot trust Shapes to have an area. You would have to catch this exception inside the AreaAgreggator class:

"Subclasses should be substitutable for their base classes."

You can see this is a problem by trying to use a Line in your Application class.

Fixing the Issue

4. Interface Segregation (ISP)

Add a draw() method to the Shape interface and implement it in each Shape. For now this method can just print the name of the class (e.g Rectangle, Circle, ...).

Identifying the Issue

Realize that your AreaAggregator class now depends on something that knows how to draw itself without really needing to, violating the ISP (Interface Segregation Principle):

Many client-specific interfaces are better than one general-purpose interface.

Fixing the Issue

5. Dependency Inversion (DIP)

Identifying the Issue

  List<House> houses = new ArrayList<>();
  houses.add(new House(50));
  houses.add(new House(150));

  City city = new City(houses);

  AreaStringOutputter cityStringOutputter = new AreaStringOutputter(city);
  AreaXMLOutputter cityXmlOutputter = new AreaXMLOutputter(city);

Having AreaStringOutputter depend on AreaAggregator makes it a not very reusable solution -- e.g., it can't be used to output a sum of areas provided by alternative implementations.

This happens because we are violating the DIP (Dependency Inversion Principle):

"High-level modules should not depend on low-level modules. Both should depend on abstractions."

Fixing the Issue

In the end, your main() method in the Application class should look something like this:

public static void main(String[] args) {
  AreaAggregator aggregator = new AreaAggregator();

  aggregator.addShape(new Square(10));
  aggregator.addShape(new Circle(5));
  aggregator.addShape(new Circle(2));
  aggregator.addShape(new Ellipse(2, 3));
  aggregator.addShape(new Rectangle(10, 5));
  aggregator.addShape(new Triangle(10, 2));
  aggregator.addShape(new House(100));

  AreaStringOutputter stringOutputter = new AreaStringOutputter(aggregator);
  AreaXMLOutputter xmlOutputter = new AreaXMLOutputter(aggregator);

  System.out.println(stringOutputter.output());
  System.out.println(xmlOutputter.output());

  List<House> houses = new ArrayList<>();
  houses.add(new House(50));
  houses.add(new House(150));

  City city = new City(houses);

  AreaStringOutputter cityStringOutputter = new AreaStringOutputter(city);
  AreaXMLOutputter cityXmlOutputter = new AreaXMLOutputter(city);

  System.out.println(cityStringOutputter.output());
  System.out.println(cityXmlOutputter.output());
}

And print:

Sum of areas: 369.9557428756428
<area>369.9557428756428</area>
Sum of areas: 200.0
<area>200.0</area>

6. Time for Heroes

Open the Hero code you created in a previous class and analyze it, looking for possible OOP principles violations.