-3
\$\begingroup\$

This code works but I don't know if I am doing this correctly. I want to have a do-while loop for the menu and app. How could I improve this code?

import java.util.Scanner; // import scanner




public class CalcPart3 { //class definition



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

        char result = 0; //define and declare variables

        double firstNumber = 0; //define double

        double secondNumber = 0; //define double

        double answer = 0; //define double



        do {


        System.out.println("Calculator Program\n"); //Text for user

        System.out.println("Choose how to proceed:\n"); //Text for user

        System.out.println("A. Add\n"); //Text for user

        System.out.println("B. Subtract\n"); //Text for user

        System.out.println("C. Multiply\n"); //Text for user

        System.out.println("D. Divide\n"); //Text for user

        System.out.println("X. Exit\n");



        Scanner userInput = new Scanner(System.in); //create new Scanner

        result = userInput.next().charAt(0); //Take only the first letter entered


        switch (result) {
                case 'a':
                case'A': 
                    System.out.println("Please enter the first number: ");
                    firstNumber = userInput.nextDouble(); //redefine first number
                    System.out.println("Please enter the second number: ");
                    secondNumber = userInput.nextDouble(); //redefine second number
                    answer = firstNumber + secondNumber; //algorithim
                    System.out.println("Answer: " + firstNumber + " + " + secondNumber + " = " + answer);//print answer
                case 'b':
                case 'B':    
                    System.out.println("Please enter the first number: ");
                    firstNumber = userInput.nextDouble(); //redefine first number
                    System.out.println("Please enter the second number: ");
                    secondNumber = userInput.nextDouble(); //redefine second number
                    answer = firstNumber - secondNumber; //algorithim
                    System.out.println("Answer: " + firstNumber + " - " + secondNumber + " = " + answer);//print answer
                case 'c':
                case 'C':
                    System.out.println("Please enter the first number: ");
                    firstNumber = userInput.nextDouble(); //redefine first number
                    System.out.println("Please enter the second number: ");
                    secondNumber = userInput.nextDouble(); //redefine second number
                    answer = firstNumber * secondNumber; //algorithim
                    System.out.println("Answer: " + firstNumber + " * " + secondNumber + " = " + answer);//print answer
                case 'd':
                case 'D':
                    System.out.println("Please enter the first number: ");
                    firstNumber = userInput.nextDouble(); //redefine first number
                    System.out.println("Please enter the second number: ");
                    secondNumber = userInput.nextDouble(); //redefine second number
                    answer = firstNumber / secondNumber; //algorithim
                    System.out.println("Answer: " + firstNumber + " / " + secondNumber + " = " + answer);//print answer
                case 'x':
                case 'X':    
                    System.exit(0);
                          }
 }

while (result == 'A' || result == 'B'|| result == 'C' || result == 'D' || result == 'a' || result == 'b'|| result == 'c' || result == 'd'
                );
    }


}
\$\endgroup\$
  • \$\begingroup\$ It does what I want. It works. My issue is the while on the bottom and I want to see how I can improve that. I believe this follows all the rules. \$\endgroup\$ – JOHNSMITH8338 Mar 15 '16 at 0:55
  • 1
    \$\begingroup\$ soo... I see a do-while there, which suggests that either you got it to work all by yourself (congrats) or this code doesn't do what you want. Or alternatively the last sentence in your question is misplaced. In either case, you should add some explanation as to what your code does (and should do) and maybe what you expect out of the review... \$\endgroup\$ – Vogel612 Mar 15 '16 at 0:59
  • \$\begingroup\$ Sorry man my first time using this site. Basically my code technically works if I run it in a compiler but the while is extremely long because I include both A and a etc. Is there any way to make it shorter? Appreciate any advice thrown my way \$\endgroup\$ – JOHNSMITH8338 Mar 15 '16 at 1:29
  • \$\begingroup\$ It sounds like you just want a more elegantly written calculator program that works like what you have. Is that right? \$\endgroup\$ – 200_success Mar 15 '16 at 1:55
  • 3
    \$\begingroup\$ No break statement in the cases of the switch. This probably doesn't work as intended. \$\endgroup\$ – janos Mar 15 '16 at 6:12
5
\$\begingroup\$

Style

The formatting of the code makes it difficult to read. There are too many blank lines, braces are not aligned very well. There are quick online tools you can use for formatting your correctly. I looked for an online beautifier for Java but could not find one. Your Java IDE should have something like that, though. In a pinch, you can also use a JavaScript or PHP beautifier, since they are similar to Java in style. I used http://jsbeautifier.org/ on your code and it looked much better already, with practically no effort.


Comments

Nearly every line of code has a comment next to it, saying what the code does. Code comments should be rare and appropriate. In many cases (yours included) the code reads easily enough to tell what it does without having comments. Here is a good article on the topic of code comments:

Code comments: A quick guide on when (and when not) to use them

PluralSight, By Zoran Horvat on July 29, 2015

Many of your comments are very redundant, for example import java.util.Scanner; // import scanner and double firstNumber = 0; //define double. I would suggest to remove all the comments, then read your code, top to bottom, and identify if any particular section is unclear and warrants commenting on them. Chances are, you will not find many.


So now that the code looks much cleaner, let's have a look at it.

Case comparison of characters/strings

In a few places, you are comparing against different cases of the same letters. You could commonly simplify this by using toUpperCase() or toLowerCase() methods on the result for instance...

Also, as @janos has remarked...

No break statement in the cases of the switch. This probably doesn't work as intended.

See The switch Statement documentation.

result = Character.toUpperCase(result);
switch (result) {
case 'A':
    // ...
    break;
case 'B':
    // ...
    break;
case 'C':
    //...
    break;
// etc.

You can also use this same comparison logic further down, as result will not have changed.

while (result == 'A' || result == 'B' || result == 'C' || result == 'D') 

Formatting output

You would benefit from reading Formatting from The Java™ Tutorials. This will help you make nicely formatted output which is also easier to read in code. It will also help you avoid having to concatenate strings and values together with + operator (such as this):

System.out.println("Answer: " + firstNumber + " + " + secondNumber + " = " + answer);

Instead you can use System.out.format() (or System.out.printf(), they are the same).

System.out.format("Answer: %d + %d = %d", firstNumber, secondNumber, answer);

The %d is a placeholder for a numeric value (i.e., digit). You simply supply the variables/arguments after the string expression in the order they appear, and then they are substituted into the string once the code runs.

You can also use it for multi-line strings, for example, and only call the (expensive) System.out once, instead of 7 times:

    do {
        System.out.format(
            "Calculator Program\n\n" +
            "Choose how to proceed:\n\n" + 
            "A. Add\n\n" +
            "B. Subtract\n\n" +
            "C. Multiply\n\n" +
            "D. Divide\n\n" +
            "X. Exit\n\n"
        ); 

Note that System.out.format() does not add a newline automatically, so I doubled the \n to preserve your formatting.


Next steps?

There are many other calculator programs in Java on Code Review, perhaps have a look:

Java Calculator posts

Since you are starting to learn to make small programs like this, perhaps now would be a good time to learn about extracting this out of the main method (which should ideally be only an entry point for your program) into a class of its own, with methods that are relevant (such as a method for each of Add, Subtract, Multiply, Divide, for instance).


Your code with the suggestions applied:

import java.util.Scanner; 

public class CalcPart3 { 

    public static void main(String[] args) { 
        char result = 0; 
        double firstNumber = 0; 
        double secondNumber = 0; 
        double answer = 0; 

        do {
            System.out.format(
                "Calculator Program\n\n" +
                "Choose how to proceed:\n\n" + 
                "A. Add\n\n" +
                "B. Subtract\n\n" +
                "C. Multiply\n\n" +
                "D. Divide\n\n" +
                "X. Exit\n\n"
            ); 

            Scanner userInput = new Scanner(System.in); 
            result = userInput.next().charAt(0); 
            result = Character.toUpperCase(result);

            switch (result) {
            case 'A':
                System.out.println("Please enter the first number: ");
                firstNumber = userInput.nextDouble(); 
                System.out.println("Please enter the second number: ");
                secondNumber = userInput.nextDouble(); 
                answer = firstNumber + secondNumber; 
                System.out.format("Answer: %d + %d = %d", firstNumber, secondNumber, answer);
                break;
            case 'B':
                System.out.println("Please enter the first number: ");
                firstNumber = userInput.nextDouble(); 
                System.out.println("Please enter the second number: ");
                secondNumber = userInput.nextDouble(); 
                answer = firstNumber - secondNumber; 
                System.out.format("Answer: %d + %d = %d", firstNumber, secondNumber, answer); 
                break;
            case 'C':
                System.out.println("Please enter the first number: ");
                firstNumber = userInput.nextDouble(); 
                System.out.println("Please enter the second number: ");
                secondNumber = userInput.nextDouble(); 
                answer = firstNumber * secondNumber; 
                System.out.format("Answer: %d + %d = %d", firstNumber, secondNumber, answer);
                break;
            case 'D':
                System.out.println("Please enter the first number: ");
                firstNumber = userInput.nextDouble(); 
                System.out.println("Please enter the second number: ");
                secondNumber = userInput.nextDouble(); 
                answer = firstNumber / secondNumber; 
                System.out.format("Answer: %d + %d = %d", firstNumber, secondNumber, answer);
                break;
            case 'X':
                System.exit(0);
            }
        } while (result == 'A' || result == 'B' || result == 'C' || result == 'D');
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Hey I just want to thank you so much. I'm obviously brand new to coding java (a month in) and your post really really helped me. Thank you!! \$\endgroup\$ – JOHNSMITH8338 Mar 16 '16 at 19:14
  • \$\begingroup\$ I'm glad I could help you improve! Feel welcome to post more questions once you write more code and apply these techniques :-) \$\endgroup\$ – Phrancis Mar 16 '16 at 19:21
  • \$\begingroup\$ I have one quick question if you don't mind. How did you choose where to place the do statement and brackets and the while statement? \$\endgroup\$ – JOHNSMITH8338 Mar 16 '16 at 19:47
  • \$\begingroup\$ In general, whenever you open a curly bracket, you will want to start on a new line, then indent once to the right (e.g. 4 spaces), and then reduce the indentation once you close that curly bracket. Here is the "official" Java style guide when it comes to statements like that: 7 - Statements \$\endgroup\$ – Phrancis Mar 16 '16 at 20:07

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.