Shape Calculator in Java

This is my first time putting up a completed program onto the web and I was hoping to get some feedback on these areas:

• Commenting (Too much, not enough or do I need to simplify anything)

• Structure (I guess does it seem reasonably tidy, presentable and laid out well)

• Naming (Are my variables well named, are sensible?)

These are the main areas that trouble me but any feedback is welcome.

I believe all those "Println" at the start are too much already and I could probably reduce the amount while keeping the same layout.

```
public class ShapeCalculator {

public static void main(String[] args) {

System.out.println("Please Enter a number beetween 1 and 9 to make a decision or 10 to Quit\n");
System.out.println("For regular and irreglar shapes choose between 1 and 6 as shown below");
System.out.println("1 = Equalatrial Triangle");
System.out.println("2 = Square");
System.out.println("3 = Pentagon");
System.out.println("4 = Hexagon\n");
System.out.println("5 = Rectangle");
System.out.println("6 = Circle\n");
System.out.println("For Solid shapes choose between 7 and 9 as shown below");
System.out.println("7 = Cube");
System.out.println("8 = Cylinder");
System.out.println("9 = Sphere\n");
System.out.println("Select 10 to Quit\n");

Scanner scan = new Scanner(System.in); //looks for and takes user input

loop: while (true) { //loop positioned here will allow flexibility of user decisions
int decision = scan.nextInt(); //user input will be in the form of integers
switch (decision) { //selection based user input

case 1:
// Decision 1 Equalatrial Triangle
System.out.println("You selected Equalatrial Triangle"); //confirm user choice
System.out.println("Enter your number to represent the 3 sides"); //Prompt user for input
double triSide = scan.nextDouble(); // get user input

double triPerm = triSide * 3; //input Times 3 will get the

-
Commenting (Too much, not enough or do I need to simplify anything)

(shamelessly copying my opinion about comments from another answer with some slight modifications...)

@rolfl posted an excellent answer to another Java question about two months ago regarding comments and Javadoc, and I feel you should definitely have a read at that. Essentially, comments should explain the why, not the how, and if you can avoid using them through the use of properly-named variables or easy-to-understand code constructs, that is generally the more favored approach outside of academic assignments.

Looking at your comments in particular, most of them simply describe the how... // Decision 9 Sphere, // Quit, // Exit Program, // Wrong decision doesn't value-add your code because it is already obvious what those lines are doing. It's better to remove redundant comments so that they don't end up being potentially misleading in the future.

-
Structure (I guess does it seem reasonably tidy, presentable and laid out well)

I can at least give a point for that. :) Even though it's one huge method, it is altogether tidy and laid out well. As mentioned in the earlier two answers, you can somewhat improve your code by breaking down into methods, or even an object-oriented (OO) approach to perform the same tasks.

-
Naming (Are my variables well named, are sensible?)

You get another point here too. However, I will always prefer to spell out names in full: sphereR, sphereVol, sphereSurf can be rewritten as sphereRadius, sphereVolume, and sphereSurfaceArea. Shaving milliseconds from the extra processing required to automagically unravel short-formed names goes a long way in making your code readable and understandable. :)

Further improvements

Similar questions to yours have surfaced in the past before, where a menu is presented to a user in the command-line interface (CLI), an option is read, and then some specific operations are performed on additional inputs.

One cleaner solution for generating the menu part and to separate different handling per option is to itemize said options into an enum type containing the logic per enum value. The benefit of this is that you eliminate the need to type a bunch of System.out.println("1..."); statements, which will otherwise be done with a loop construct.

Then, to handle the varying inputs you require (some shapes require a single dimension input, others two), you can consider mapping each user prompt into a double input for the enum values. For example, if you need to prompt the user twice, a method can then return a two-element double[] array for you to perform your calculations.

Roughly putting both together...

public class MyClass {

    enum Shape {
        SQUARE("Enter side"),
        RECTANGLE("Enter long side", "Enter short side");

        final String[] prompts;

        private Shape(String... prompts) {
            this.prompts = prompts;
        }

        private void doCalculations(double[] inputs) {
            // this can either be a switch statement on the enum values,
            // or an abstract method that will be implemented individually
        }
    }

    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            printMenu();
            Shape shape = Shape.values()[getIntegerInput(scanner)];
            shape.doCalculations(getInputs(scanner, shape.prompts));
        }
    }

    private static void printMenu() {
        // there's also a Stream-based Java 8 way too
        for (Shape shape : Shape.values()) {
            System.out.printf("%d) %s\n", shape.ordinal(), shape);
        }
    }

    private static double[] getInputs(Scanner scanner, String... prompts) {
        // there's also a Stream-based Java 8 way too
        double[] result = new double[prompts.length];
        for (int i = 0; i < prompts.length; i++) {
            System.out.println(prompts[i]);
            result[i] = getDoubleInput(scanner);
        }
        return result;
    }

    private static int getIntegerInput(Scanner scanner) {
        // this will return 1, 2, 3, etc. after validation
    }

    private static double getDoubleInput(Scanner scanner) {
        // this will return a double value after validation
    }
}

The other benefit of the enum type here is that you also have a clearer segregation of your calculation logic and the user input. Notice that Shape.doCalculations() only takes in a double[] array, so it also becomes almost trivial to do unit testing (if you replace the return value accordingly too).

StackExchange Code Review Q#96266, answer score: 3