6
\$\begingroup\$

So I made this calculator the other day as a small project. I just wanted to know if I remembered the fundamentals. It works fine from the looks of it, but I know there are always ways to improve.

    package Main;
    import java.util.*;
    public class calc {
@SuppressWarnings("resource")
private static void circle() {
    // TODO Auto-generated method stub
    double a_c = 0; //a_c = area of circle

    System.out.println("Please enter the radius of the circle."); 

    Scanner read_r = new Scanner(System.in); // This is the scanner that scan user input.

    double r = read_r.nextDouble(); // This is the radius the user enters.

    a_c = Math.PI * r *r ; //a_c stands for area of circle followed by equation.

    System.out.print("The area of the circle equals: ");

    System.out.println(a_c);// the system is printing the result; the value of a_c

    main(null);
}
@SuppressWarnings("resource")
private static void square() {
    // TODO Auto-generated method stub

    double a_s = 0; // a_s = area of square initial value

    System.out.println("Please enter the value of the length of one side of the square."); 

    Scanner read_s = new Scanner(System.in); // This is the scanner that scan user input.

    double s = read_s.nextDouble(); // This is the side length the user enters.

    a_s = s*s; //equation for square area = s^2

    System.out.print("The area of the square equals: ");

    System.out.println(a_s);


    main(null);
}
@SuppressWarnings("resource")
private static void triangle() {
    // TODO Auto-generated method stub
    double a_t = 0; //a_t = area of triangle initial value

    System.out.println("Please enter the height of your triangle."); 

    Scanner read_h = new Scanner(System.in); // This is the scanner that will read height.

    int h = read_h.nextInt(); // This is the height the user enters.

    System.out.println("Please enter the value for the base of your triangle");

    Scanner read_b = new Scanner(System.in); // This is the scanner that reads base

    Double b = read_b.nextDouble(); // This is the base the user enters

    a_t = .5*b*h;

    System.out.print("The area of the traingle equals: ");

    System.out.println(a_t);

    main(null);
}
@SuppressWarnings("resource")
private static void rectangle() {
    // TODO Auto-generated method stub
    double a_r = 0;// a_r = area of rectangle initial value

    System.out.println("Please enter the width of your rectangle."); 

    Scanner read_w = new Scanner(System.in); // This is the scanner that scan user input.

    double w = read_w.nextInt(); // This is the width value the user enters.

    System.out.println("Please enter a value for the height of your triangle.");

    Scanner read_h = new Scanner(System.in);

    double h = read_h.nextDouble();

    a_r = h*w;

    System.out.print("The area of the rectangle equals: ");

    System.out.println(a_r);

    main(null);
}
private static void quit(){
    System.out.println("Quiting....");

    System.exit(0);

}
@SuppressWarnings("resource")
public static void main(String [] args1){

    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println("1. Area of a Circle");

    System.out.println("2. Area of a Square");

    System.out.println("3. Area of a Triangle");

    System.out.println("4. Area of a Rectangle");

    System.out.println("5. Quit");

    Scanner readc = new Scanner(System.in);

    int c = readc.nextInt(); //This is the choice method 1-4

switch (c){
case 1:
    circle();
    break;
case 2:
    square();
    break;
case 3:
    triangle();
case 4:
    rectangle();
case 5:
    quit();
    break;
default:
    System.out.println("I'm sorry, you didn't select a function.");
    System.out.println("Please choose again");
    main(null);
    }
 }

}

\$\endgroup\$
7
\$\begingroup\$

First of all, well done on building a working area calculator. Lets see how we can improve it!

To start off, you have quite a few indentation problems. The definition of the class should not be indented at all, and the methods inside the class should be indented once. For each nesting level, you indent another time.

Also, your class is named calc, but Java classes are supposed to be PascalCase, meaning it should be Calc, with an uppercase c.

Functions

You are using too much vertical space. Not every line needs to have an empty line all around it. Try to group blocks of related lines of code together.

Most, if not all of your functions, do more than one thing. In programming we strive for functions that perform a single task and do so very well. For example, your main function is also in charge of how the user interface is presented to the user. This should be offloaded to another function like so:

private static void showUserOptions() {
    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println("1. Area of a Circle");
    System.out.println("2. Area of a Square");
    System.out.println("3. Area of a Triangle");
    System.out.println("4. Area of a Rectangle");
    System.out.println("5. Quit");
}

Now we can simply call this function from the main method. As you can see, I have also grouped the System.out.println() statements that specify the options very close to each other. This helps with readability a lot.

Your functions also contain a lot of code duplication, such as printing the area of the selected shape and creating a new Scanner instance in every function, which is totally unnecessary and can be replaced with a single static instance like so:

private static Scanner inputScanner = new Scanner(System.in);

Furthermore, your variable names are not very clear. a_r is a useless variable name. How about rectangleArea?

User Input

For the user input of your program, we can user Java enums:

private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }

Here I have created 4 enum values for the different shapes, a QUIT enum value for the quit command and an INVALID value for any user input that is invalid.

Now we do have to change the showUserOptions() function a bit to make it correspond with the new enum values. This is done as follows:

private static void showUserOptions() {
    System.out.println("What would you like to do? Type the number of your desired function.");

    System.out.println(UserInput.CIRCLE.ordinal() + ". Area of a Circle");
    System.out.println(UserInput.SQUARE.ordinal() + ". Area of a Square");
    System.out.println(UserInput.TRIANGLE.ordinal() + ". Area of a Triangle");
    System.out.println(UserInput.RECTANGLE.ordinal() + ". Area of a Rectangle");
    System.out.println(UserInput.QUIT.ordinal() + ". Quit");
}

Calling ordinal() on an enum will give you the index it has in the list of enum values. Meaning that UserInput.SQUARE.ordinal() will result in 0 and UserInput.QUIT.ordinal() will result in 4.

To retrieve an actual enum object from the integer input provided by the user, the following code can be used:

int input = inputScanner.nextInt();
UserInput userInput = UserInput.values()[input];

UserInput.values() creates an array of the enum values, which looks like this: [CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID]. Indexing it with the integer input from the user gives us the actual enum value. The UserInput.values() call actually creates a new array every time it is called, so we will be caching it as a static variable right underneath the declaration of the enum itself:

private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }
private static UserInput[] userInputs = UserInput.values();

Since the user can enter any integer value, even ones that are outside the range of the userInputs() array, we create a special function to deal with this and ensure that we always get a UserInput enum:

private static UserInput getUserInputFromInt(int userInput) {
    if (userInput >= 0 && userInput < userInputs.length) {
        return userInputs[userInput];
    }

    return UserInput.INVALID;
}

Since your code contained a lot of duplication of retrieving user input, I have created this function:

private static double askUserForValue(String valueName, UserInput userInput) {
    System.out.println(String.format("Please enter the %s of your %s.", valueName, userInput.toString().toLowerCase()));

    return inputScanner.nextDouble();
}

Which we can call like this:

double radius = askUserForValue("radius", userInput);

To ask the user to provide us the radius of a circle.

Main method

The main method of your program should be concise and clear. Also it should definitely not be called manually from other functions. These other functions have no business calling the main method, as they should only be looking to fulfill their single task. I would therefore suggest a rewrite of your main() function:

public static void main(String[] args) {
    while (true) {
        showUserOptions();

        UserInput userInput = getUserInputFromInt(inputScanner.nextInt());
        actOnUserInput(userInput);
    }
}

A few things stand out here. I have renamed your scanner to inputScanner, as readc isn't that good of a name for a Scanner object. Also I am calling a method that we have not yet defined, called actOnUserInput(). I will show this function in detail below. Finally, I have wrapped these two function calls in a while loop, to ensure that they will repeatedly be called. This makes it so that we can remove the calls to main() from all other places in the code.

Object Oriented Programming

Why not practice some object oriented programming while we are at it?

What we are dealing with here is a bunch of shapes. This allows us to create an abstraction of a Shape object. This is achieved by defining a common interface that all shapes share:

interface Shape {
    public double getArea();
}

Now that we have the Shape interface, we can create some concrete implementations of it. Lets start out with the circle object:

private static class Circle implements Shape {
    double radius;

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

    @Override
    public double getArea() {
        return Math.PI * this.radius * this.radius;
    }
}

Now we can do the same for the triangle and rectangle shapes:

private static class Triangle implements Shape {
    double height;
    double base;

    public Triangle(double height, double base) {
        this.height = height;
        this.base = base;
    }

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

private static class Rectangle implements Shape {
    double length;
    double width;

    public Rectangle(double length, double width) {
        this.length = length;
        this.width = width;
    }

    @Override
    public double getArea() {
        return this.length * this.width;
    }
}

All that is missing now is the Square class. However, in mathematics, a square is a subclass of rectangle. Implementing this using inheritance would look like this:

private static class Square extends Rectangle {
    public Square(double sideLength) {
        super(sideLength, sideLength);
    }
}

Here, the square class only calls the parent constructor using the same length parameter for the width and height. The getArea() functionality is inherited from the Rectangle class and doesn't need to be implemented again.

Whether or not it is a good idea to have Square inherit from Rectangle is debatable and often discouraged, since inheritance can become quite messy. However, for the sake of this example I believe it is justified.

Now that we have defined a bunch of concrete Shape implementations, we would like to create them!

We can use the UserInput enum to determine which type of shape we want to create. This can be put into a single factory-like function that will simply return a Shape object:

private static Shape createShapeFromUserInput(UserInput userInput) {
    switch (userInput) {
        case CIRCLE:
            double radius = askUserForValue("radius", userInput);

            return new Circle(radius);
        case SQUARE:
            double sideLength = askUserForValue("side length", userInput);

            return new Square(sideLength);
        case TRIANGLE:
            double height = askUserForValue("height", userInput);
            double base = askUserForValue("base", userInput);

            return new Triangle(height, base);
        case RECTANGLE:
            double length = askUserForValue("length", userInput);
            double width = askUserForValue("width", userInput);

            return new Rectangle(length, width);
    }

    return null;
}

Now we can incorporate this function into the yet to be defined actOnUserInput() function:

private static void actOnUserInput(UserInput userInput) {
    switch (userInput) {
        case CIRCLE:
        case SQUARE:
        case TRIANGLE:
        case RECTANGLE:
            Shape shape = createShapeFromUserInput(userInput);

            if (shape != null) {
                System.out.println("The area is equal to: " + String.valueOf(shape.getArea()));
            }
            break;
        case QUIT:
            quit();
            break;
        case INVALID:
            System.out.println("I'm sorry, you didn't select a function.");
            System.out.println("Please choose again");
            break;
    }
}

Here we can see the purpose of the Shape interface come through. After calling createShapeFromUserInput(), we don't care whether a Rectangle, Circle, or whatever type of shape is returned, only that it is a Shape instance. This is because now we can simply call the getArea() method defined on the Shape interface and rely on that.

The entire program now looks like this:

import java.util.Scanner;

public class Calc {
    private enum UserInput { CIRCLE, SQUARE, TRIANGLE, RECTANGLE, QUIT, INVALID }
    private static UserInput[] userInputs = UserInput.values();

    private static Scanner inputScanner = new Scanner(System.in);

    public static void main(String[] args) {
        while (true) {
            showUserOptions();

            UserInput userInput = getUserInputFromInt(inputScanner.nextInt());
            actOnUserInput(userInput);
        }
    }

    private static void showUserOptions() {
        System.out.println("What would you like to do? Type the number of your desired function.");

        System.out.println(UserInput.CIRCLE.ordinal() + ". Area of a Circle");
        System.out.println(UserInput.SQUARE.ordinal() + ". Area of a Square");
        System.out.println(UserInput.TRIANGLE.ordinal() + ". Area of a Triangle");
        System.out.println(UserInput.RECTANGLE.ordinal() + ". Area of a Rectangle");
        System.out.println(UserInput.QUIT.ordinal() + ". Quit");
    }

    private static UserInput getUserInputFromInt(int userInput) {
        if (userInput >= 0 && userInput < userInputs.length) {
            return userInputs[userInput];
        }

        return UserInput.INVALID;
    }

    private static void actOnUserInput(UserInput userInput) {
        switch (userInput) {
            case CIRCLE:
            case SQUARE:
            case TRIANGLE:
            case RECTANGLE:
                Shape shape = createShapeFromUserInput(userInput);

                if (shape != null) {
                    System.out.println("The area is equal to: " + String.valueOf(shape.getArea()));
                }
                break;
            case QUIT:
                quit();
                break;
            case INVALID:
                System.out.println("I'm sorry, you didn't select a function.");
                System.out.println("Please choose again");
                break;
        }
    }

    private static Shape createShapeFromUserInput(UserInput userInput) {
        switch (userInput) {
            case CIRCLE:
                double radius = askUserForValue("radius", userInput);

                return new Circle(radius);
            case SQUARE:
                double sideLength = askUserForValue("side length", userInput);

                return new Square(sideLength);
            case TRIANGLE:
                double height = askUserForValue("height", userInput);
                double base = askUserForValue("base", userInput);

                return new Triangle(height, base);
            case RECTANGLE:
                double length = askUserForValue("length", userInput);
                double width = askUserForValue("width", userInput);

                return new Rectangle(length, width);
        }

        return null;
    }

    private static double askUserForValue(String valueName, UserInput userInput) {
        System.out.println(String.format("Please enter the %s of your %s.", valueName, userInput.toString().toLowerCase()));

        return inputScanner.nextDouble();
    }

    private static void quit() {
        System.out.println("Quiting....");

        System.exit(0);

    }

    private interface Shape {
        double getArea();
    }

    private static class Circle implements Shape {
        double radius;

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

        @Override
        public double getArea() {
            return Math.PI * this.radius * this.radius;
        }
    }

    private static class Triangle implements Shape {
        double height;
        double base;

        public Triangle(double height, double base) {
            this.height = height;
            this.base = base;
        }

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

    private static class Rectangle implements Shape {
        double length;
        double width;

        public Rectangle(double length, double width) {
            this.length = length;
            this.width = width;
        }

        @Override
        public double getArea() {
            return this.length * this.width;
        }
    }

    private static class Square extends Rectangle {
        public Square(double sideLength) {
            super(sideLength, sideLength);
        }
    }
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ I could not have asked for a better response. Thank you for taking the time to review my code so thoroughly. Huge amounts of valuable information here that I will most definitely take note of when fixing this project and creating future projects. You rock ! \$\endgroup\$ – Edward Ponce Jun 8 '16 at 12:51
2
\$\begingroup\$

Your code looks almost completely perfect. There is only one thing I would say: There is no need to declare a Scanner for System.in every single time you would look for something. You only need to declare it once. So, if I were to edit your code, here's how I would do it:

package Main;
import java.util.*;
public class calc {

    private Scanner scanner;

    @SuppressWarnings("resource")
    private static void circle() {
        double a_c = 0; //a_c = area of circle
        System.out.println("Please enter the radius of the circle."); 
        double r = scanner.nextDouble(); // This is the radius the user enters.
        a_c = Math.PI * r *r ; //a_c stands for area of circle followed by equation.
        System.out.print("The area of the circle equals: ");
        System.out.println(a_c);// the system is printing the result; the value of a_c
        main(null);
    }
    @SuppressWarnings("resource")
    private static void square() {
        double a_s = 0; // a_s = area of square initial value
        System.out.println("Please enter the value of the length of one side of the square."); 
        double s = scanner.nextDouble(); // This is the side length the user enters.
        a_s = s*s; //equation for square area = s^2
        System.out.print("The area of the square equals: ");
        System.out.println(a_s);
        main(null);
    }

    @SuppressWarnings("resource")
    private static void triangle() {
        double a_t = 0; //a_t = area of triangle initial value
        System.out.println("Please enter the height of your triangle."); 
        int h = scanner.nextInt(); // This is the height the user enters.
        System.out.println("Please enter the value for the base of your triangle");
        Double b = scanner.nextDouble(); // This is the base the user enters
        a_t = .5*b*h;
        System.out.print("The area of the traingle equals: ");
        System.out.println(a_t);
        main(null);
        }

    @SuppressWarnings("resource")
    private static void rectangle() {
        double a_r = 0;// a_r = area of rectangle initial value
        System.out.println("Please enter the width of your rectangle."); 
        double w = scanner.nextInt(); // This is the width value the user enters.
        System.out.println("Please enter a value for the height of your triangle.");
        double h = scanner.nextDouble();
        a_r = h*w;
        System.out.print("The area of the rectangle equals: ");
        System.out.println(a_r);
        main(null);
    }
    private static void quit(){
        System.out.println("Quiting....");
        System.exit(0);
    }
    @SuppressWarnings("resource")
    public static void main(String [] args1){
    scanner = new Scanner(System.in);
    System.out.println("What would you like to do? Type the number of your desired function.");
    System.out.println("1. Area of a Circle");
    System.out.println("2. Area of a Square");
    System.out.println("3. Area of a Triangle");
    System.out.println("4. Area of a Rectangle");
    System.out.println("5. Quit");
    int c = scanner.nextInt(); //This is the choice method 1-5

switch (c){
case 1:
    circle();
    break;
case 2:
    square();
    break;
case 3:
    triangle();
case 4:
    rectangle();
case 5:
    quit();
    break;
default:
    System.out.println("I'm sorry, you didn't select a function.");
    System.out.println("Please choose again");
    main(null);
    }
}
}

Because you call the main method recursively, the scanner will actually be defined many times. However, this code does remove the unnecessary duplicate Scanner objects

| improve this answer | |
\$\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.