7
\$\begingroup\$

This was my second project as a beginner in Java programming. Please provide feedback/ tips on my program.

import java.util.Scanner;

public class Area {
    public static void main(String[] args) {

        Scanner Area = new Scanner (System.in); 

        double length, width,totalArea, height; 
        String shape, unit;
        int loop = 0; 

        while (loop == 0){ 

        System.out.println("Which shape do you want to find the area?");
        shape = Area. next(); 

        switch (shape) { 

        case "rectangle": 
            System.out.println("Please enter the leghth of the rectangle: ");
            length = Area. nextDouble (); 
            System.out.println("Please enter the width of the rectangle: ");
            width = Area. nextDouble(); 
            System.out.println("What is the unit of the shape?");
            unit = Area.next();  
            totalArea = length*width; 
            System.out.println(totalArea+unit);
            break; 

        case "square" : 
            System.out.println("Please enter the leghth of the square: ");
            length = Area. nextDouble (); 
            System.out.println("Please enter the width of the square: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = length*width; 
            System.out.println(totalArea+unit);
            break; 

        case "triangle" : 
            System.out.println("Please enter the base of the triangle: ");
            length = Area. nextDouble (); 
            System.out.println("Please enter the height of the triangle: ");
            width = Area. nextDouble(); 
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = ((length*width)/2); 
            System.out.println(totalArea+unit);
            break;

        case "parallelogram" : 
            System.out.println("Please enter the base of the parallelogram: ");
            length = Area. nextDouble (); 
            System.out.println("Please enter the height of the parallelogram: ");
            width = Area. nextDouble(); 
            System.out.println("What is the unit of the shape?");
            unit = Area.next();  
            totalArea = length*width; 
            System.out.println(totalArea+unit);
            break; 

        case "circle" : 
            System.out.println("Please enter the radius of the circle: ");
            length = Area. nextDouble(); 
            System.out.println("What is the unit of the shape?");
            unit = Area.next();  
            totalArea = 3.14*(Math.pow(length, 2));
            System.out.println(totalArea+unit);
            break; 

        case "rhombus" : 
            System.out.println("Please enter the first diagonal of the rhombus: "); 
            length = Area. nextDouble(); 
            System.out.println("Please enter the second diagonal of the rhombus: ");
            width = Area. nextDouble(); 
            System.out.println("What is the unit of the shape?");
            unit = Area.next();  
            totalArea = ((length*width)/2);
            System.out.println(totalArea+unit);
            break; 

        case "trapezoid" :
            System.out.println("Please enter the first base of the trapezoid: "); 
            length = Area. nextDouble(); 
            System.out.println("Please enter the second base of the trapezoid: ");
            width = Area. nextDouble(); 
            System.out.println("Please enter the height of the trapezoid: ");
            height = Area.nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();  
            totalArea = (((length+width)*height)/2);
            System.out.println(totalArea+unit);
            break; 

        default: 
            System.out.println("The shape you entered is not recongnized");
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Have you learned about classes and polymorphism? \$\endgroup\$ – cricket_007 Jul 25 '16 at 4:33
6
\$\begingroup\$

First, you have improved yourself. But I still see the habit of your

 int loop = 0; 

        while (loop == 0){...

structure in both the programs you have written so far (Your program doesn't actually iterate as you don't change the loop variable). Actually, while loops are faster than any other loops. On the other hand, it is also important to write more clean & clear code.

As your loop-body is not more than 5000 or more lines, it's better to use a do-while(){...} looping structure.


Polymorphism (Factory Pattern + Strategy Pattern)

You can use Polymorphism in your program to make it nice. (If you don't know what is polymorphism)

Switch cases are good but replacing switch-case with polymorphism is fantastic. Look, you have different things to do in each switch case. Basically, different shapes formed with different ways.

Shape Interface

public interface Shape {

    double calculateArea();
    void buildShape(Scanner scanner);
}

ShapeClub.java (Main class)

public class ShapeClub {

    public static void main(String[] args){

        Scanner scanner = new Scanner (System.in);
        boolean retry = false;

        do{
            System.out.println("Which shape do you want to find the area?");
            Shape unknownShape = ShapeFactory.getShape(scanner.next());
            unknownShape.buildShape(scanner);

            System.out.print("Do you want to try a different shape?[Y/N]:");
            retry = scanner.next().equalsIgnoreCase("Y");
        }while (retry);
        scanner.close();
    }
}

ShapeFactory.java (Factory class)

public class ShapeFactory {

    private ShapeFactory(){}

    public static Shape getShape(String shapeName){
        switch (shapeName){
            case "rectangle":
                return new Rectangle();
            case "circle":
                return new Circle();
            default:
                return null;
        }
    }
}

Rectangle.java (Concrete class)

public class Rectangle implements Shape {

    private double length = 0.0;
    private double width = 0.0;
    private String unit ="";

    @Override
    public double calculateArea() {
        return length*width;
    }

    @Override
    public void buildShape(Scanner scanner) {
        System.out.println("Please enter the leghth of the rectangle: ");
        length = scanner.nextDouble ();
        System.out.println("Please enter the width of the rectangle: ");
        width = scanner.nextDouble();
        System.out.println("What is the unit of the shape?");
        unit = scanner.next();
        System.out.println(calculateArea()+unit);
    }
}

Circle.java (Another concrete class)

public class Circle implements Shape {

    private double radius = 0.0;
    private String unit ="";

    @Override
    public double calculateArea() {
        return Math.PI*(Math.pow(radius, 2));
    }

    @Override
    public void buildShape(Scanner scanner) {
        System.out.println("Please enter the radius of the circle: ");
        radius = scanner.nextDouble();
        System.out.println("What is the unit of the shape?");
        unit = scanner.next();
        System.out.println(calculateArea()+unit);
    }
}

Hope you understood it. :)

\$\endgroup\$
  • \$\begingroup\$ Your shape factory probably wants to return an unknown shape as a default, or you need to check the response in ShapeClub, at the moment you can end up calling buildShape on a null object \$\endgroup\$ – forsvarir Jul 25 '16 at 8:02
  • 2
    \$\begingroup\$ The statement that "while loops are faster than any other loops" is not true. The byte code generated by a traditional for loop and a while loop is the same. If you're talking about a for-each loop then there is a difference, but generally it's so negligible that it's not worth trying to optimize. Furthermore the difference between a while and do-while loop is also negligible - the byte code differs by a single command. It's highly unlikely that the type of loop will be a performance bottleneck anyway, so there is little point in bringing up this level of detail especially to a beginner. \$\endgroup\$ – D.B. Jul 27 '16 at 4:58
4
\$\begingroup\$

I noticed at first a copy-paste problem between square and rectangle shapes. To illustrate, using your program:

Which shape do you want to find the area?
 square
Please enter the leghth of the square: 
 4
Please enter the width of the square: 
 3
What is the unit of the shape?
 cm
12.0cm

Your program is asking for both the "leghth" (length) and width of the square, which should both be the same. The code for a square and a rectangle is identical, which is wrong.

This should work nicely:

System.out.println("Please enter the length of each side of the square: ");
length = Area.nextDouble ();
totalArea = length*length; 

I found a bug:

Which shape do you want to find the area?
 circle
Please enter the radius of the circle: 
 foo
Exception in thread "main" java.util.InputMismatchException
  at java.util.Scanner.throwFor(Scanner.java:864)
  at java.util.Scanner.next(Scanner.java:1485)
  at java.util.Scanner.nextDouble(Scanner.java:2413)
  at Main.main(Main.java:68)

If you are expecting only numbers as input, you should provide a way to handle this exception. Here is an example way to handle it with a do-while loop:

        case "circle" : 
            boolean isValidNumber = false;
            do {
                System.out.println("Please enter the radius of the circle: ");
                try {
                    length = Area.nextDouble();
                    isValidNumber = true;
                } catch (InputMismatchException exc) {
                    System.out.println("Invalid number input.");
                }
            } while(isValidNumber = false);

while (loop == 0)

This condition is never negated, and in your case it means the program never ends, unless forcibly done by quitting the execution, or by an exception. You should provide a mechanism for the user to quit the program.


The main method is intended to be the entry point of the program only, and not the entire program. As Crazy Ninja suggested you program would be made a lot cleaner by extracting the shapes into their own classes and such.


You should declare your variables individually for improved clarity.

    double length, width,totalArea, height; 
    String shape, unit;

Should be:

    double length;
    double width;
    double totalArea;
    double height; 
    String shape;
    String unit;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.