Shape area computer

Question

I've builded a 2D-shape area calculator. Currently it can only calculate:

• Circle
• Rectangle
• Triangle

Originally I was thinking how to encapsulate the parameter(s) into a class, Parameters, so that I could delegate the responsibility of loading those parameter(s) to a specific shape, for example, a rectangle will need a base and a height, to the Parameters class. Then finally I made it too complicated.

So instead, I try the following implementation. To focus on any better way to load the parameters and to update the area, I assumed the input of the user is correct.

Any advice about the pros and cons of my implementation is appreciated. Any other way to solve my problem is appreciated, also.

UML:

Edit: I made all setters return this so I can set a Rectangle, r, like this: r.setHeight(20.).setWidth(10.), so you don't have to care which parameter comes first.(r.getArea() will return 200).

Source Code:

Shape.java

public abstract class Shape {
    protected double area;

    public double getArea() {
        return area;
    }

    protected abstract void onAreaChange();
}

Triangle.java

public class Triangle extends Shape {
    protected double base;
    protected double height;

    public Triangle setBase(double base) {
        this.base = base;
        onAreaChange();
        return this;
    }

    public Triangle setHeight(double height) {
        this.height = height;
        onAreaChange();
        return this;
    }

    @Override
    public void onAreaChange() {
        this.area = 0.5 * this.base * this.height;
    }
}

Rectangle.java

public class Rectangle extends Shape {
    protected double height;
    protected double width;

    public Rectangle setHeight(double height) {
        this.height = height;
        onAreaChange();
        return this;
    }

    public Rectangle setWidth(double width) {
        this.width = width;
        onAreaChange();
        return this;
    }

    @Override
    public void onAreaChange() {
        this.area = this.height * this.width;
    }
}

Circle.java

public class Circle extends Shape {
    protected double radius;
    protected final double PI = 3.14159265;

    public Circle setRadius(double radius) {
        this.radius = radius;
        onAreaChange();
        return this;
    }

    @Override
    public void onAreaChange() {
        this.area = this.radius * this.radius * PI;
    }
}

Main.java

import java.util.Scanner;

public class Main {
    public static Scanner scan = new Scanner(System.in);
    public static void main(String[] args) {
        while (true) {
            System.out.println("Enter the number which you want to compute the area");
            System.out.print("(1) Triangle (2) Rectangle (3) Circle ? ");
            switch (scan.nextInt()) {
            case 1:
                Triangle triangle = new Triangle();
                System.out.print("Base: ");
                triangle.setBase(scan.nextDouble());
                System.out.print("Height: ");
                triangle.setHeight(scan.nextDouble());
                System.out.println("Area of triangle: " + triangle.getArea());
                break;
            case 2:
                Rectangle rectangle = new Rectangle();
                System.out.print("Width: ");
                rectangle.setWidth(scan.nextDouble());
                System.out.print("Height: ");
                rectangle.setHeight(scan.nextDouble());
                System.out.println("Area of rectangle: " + rectangle.getArea());
                break;
            case 3:
                Circle circle = new Circle();
                System.out.print("Radius: ");
                circle.setRadius(scan.nextDouble());
                System.out.println("Area of circle: " + circle.getArea());
                break;
            default:
                System.out.println("What do u mean?");
            }
        }
    }
}

Answer 1

I mostly agree with Indrit's answer. The way you're using the shape class doesn't do it much good. But let's look at how we need to change them to make them really useful.

First of all I'm not a fan of protected variables. The variables in the implementing classes (Triangle, ...) should just be private.
The biggest impact we can get however is by removing the area variable from the Shape class completely. Instead just calculate the area when it's requested.

this changes the Shape implementation to this:

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

The changes to the implementing classes are really easy actually. Just rename the onAreaChanged method to getArea and remove the calls to onAreaChanged() inside the setters. Also don't forget to make getArea return the area instead of assigning to a now non-existing variable. For example:

public class Triangle extends Shape {
    private double base;
    private double height;

    public Triangle setBase(double base) {
        this.base = base;
        return this;
    }

    public Triangle setHeight(double height) {
        this.height = height;
        return this;
    }

    @Override
    public double getArea() {
        return 0.5 * this.base * this.height;
    }
}

The next big thing we are going to do is move the user input for the shape-specific parameters inside each shape. Let's start by defining a new method on Shape

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

Now that the shape itself is responsible for requesting the parameters from the user we no longer need the setters either. Let's look at the new Triangle for example:

public class Triangle extends Shape {
    private double base;
    private double height;

    @Override
    public void requestParameters(){
            System.out.println("What is the base of the triangle?");
            base = scan.nextDouble();
            System.out.print("What is the height of the triangle?");
            height = scan.nextDouble();
    }

    @Override
    public double getArea() {
        return 0.5 * this.base * this.height;
    }
}

Notice now that the steps for Shape are exactly the same:

Shape shape = new /*choose shape here*/;
shape.requestParameters();
System.out.println("The area for /*oops*/ = " + shape.getArea());

Ok, so they are almost independend of the shape. We still want to print a specific message with the area of the shape.

There are 2 easy ways to fix this. Either provide a printAreaText() method inside Shape, or provide a getName() method to fill in the oops above.
Let's go for the second solution:

public abstract class Shape {
    public abstract double getArea();
    public abstract void requestParameters();
    public abstract String getName();
}

public class Triangle {
    ...
    @Override
    public String getName(){
        return "Triangle";
    }
}

The new main loop looks like this now:

public static void main(String[] args) {
    while (true) {
        System.out.println("Enter the number which you want to compute the area");
        System.out.print("(1) Triangle (2) Rectangle (3) Circle ? ");
        switch (scan.nextInt()) {
        case 1:
            Shape shape = new Triangle();
            shape.requestParameters();
            System.out.println("Area of " + shape.getName() + ": " + 
                               shape.getArea());
            break;
        case 2:
            Shape shape = new Rectangle();
            shape.requestParameters();
            System.out.println("Area of " + shape.getName() + ": " + 
                               shape.getArea());
            break;
        case 3:
            Shape shape = new Circle();
            shape.requestParameters();
            System.out.println("Area of " + shape.getName() + ": " + 
                               shape.getArea());
            break;
        default:
            System.out.println("What do u mean?");
        }
    }
}

The only thing that we want to improve here is to not repeat those steps for each shape. Let's try putting all the shapes into a list and then get the shape from the list based on the number the user has input (sounds like an index right?).

public static void main(String[] args) {
    List<Shape> shapes = new ArrayList<>();
    shapes.add(new Triangle());
    shapes.add(new Rectangle());
    shapes.add(new Circle());

    boolean userWantsToStop = false;
    while (!userWantsToStop) {
        System.out.println("Enter the number which you want to compute the area");
        System.out.println("(0) I don't want to anymore. (1) Triangle "+
                         "(2) Rectangle (3) Circle ? ");

        int choice = scan.nextInt();

        if(choice == 0){
            userWantsToStop = true;
            continue;
        }

        if(choice >= shapes.length()){
            System.out.println("What do u mean?");
            continue;
        }

        Shape shape = shapes.get(choice-1);
        shape.requestParameters();
        System.out.println("Area of " + shape.getName() + ": " + 
                               shape.getArea());
    }
}

Note the >= shapes.length() and shapes.get(choice-1) because indeces in java start at 0.

Now if we implement another shape, like Pentagon we only need to add it to the list of shapes and we're done right? Well almost. We also need to update our question. So instead, let's try to change that too:

System.out.println("Enter the number for which shape you want to compute the area.");
System.out.println("(0) I don't want to anymore.");
for(int i = 0; i < shapes.length(); i++){
    System.out.println(" (" + i + ") " + shapes.get(i).getName();
}

Good thing we already had that getName() method. I slightly changed the message and used println instead of print to make it easier on the user.

The only things left to do to improve this code is to do some input checks to make sure the user actually inputs a number for example.

Answer 2

please consider some quick feedback below:

• not clear why do you need Shape class? It looks like it is an interface but in any case how you are using your program seems that you don't need it either.

• considering your "main" are you sure do you need these classes at all ?

• if you need classes because you like playing with them, consider implementing the area as a function (and check the private instance variables) and remove the redundant 'get' prefix?

• typically it is not polite to start a server without having a civilised way to turn it off, what do you thing about adding (0) Terminate ?

• check the user message "Enter the number which you want to compute the area"

• Consider making your code more confident (how do you know that the user will insert a double?)