6
\$\begingroup\$

I am a beginner to Java and am learning about using methods. I want to make a simple calculator that uses methods effectively. I would like a review of my program, and I would also like to know if there is a better way or shortcuts to write this program better.

I would like to know at the same time about good coding style. Did I practice good coding style in my program? Where did I not practice good style? How could I improve this program so that it's in a good coding style?

import java.text.DecimalFormat;
import java.util.Scanner;

public class DoTheMath_A {

    public static void main(String[] args) {
        // First number storage
        double num1 = getFirstNumber(); // Value - returning method.

        // Second number storage
        double num2 = getSecondNumber(); // Value - returning method.

        // Displays Math Operations menu
        showMenu(); // Void - returning method

        // Math Operation choice storage
        int choice = getSelection(); // Value - returning method

        // Calculates the users numbers into an result. No output.
        double result = doTheMath(choice, num1, num2); // Value - returning
                                                        // method

        // Displays the output with formatting to show 2 decimal places.
        displayResult(choice, result, num1, num2); // Void - returning method

    } // end of main()

    // Value - returning method =======================================
    private static double getFirstNumber() {
        Scanner input = new Scanner(System.in);

        System.out.print("Enter the first number: ");
        double num1 = input.nextDouble(); // First number storage

        return num1; // Returns First number to main
    } // end of getFirstNumber()

    // Value - returning method =======================================
    private static double getSecondNumber() {
        Scanner input = new Scanner(System.in);

        System.out.print("\nEnter the second number - "
                + "(Cannot be zero for Divison): ");
        double num2 = input.nextDouble(); // Second number storage

        return num2;
    } // end of getSecondNumber()

    // Void - returning method ============================
    private static void showMenu() {
        System.out.print("\n1. Addition\n2. Subtract\n3. Multiply"
                + "\n4. Divide\n\nWhich math operation? "
                + "Enter your choice:  ");
    } // end of showMenu()

    // Value - returning method =======================================
    private static int getSelection() {
        Scanner input = new Scanner(System.in);

        int choice = input.nextInt(); // Reads User pick for math operation and
                                        // stores it into 'choice'

        switch (choice) { // Start of Switch
        case 1:

            break;
        case 2:

            break;
        case 3:

            break;
        case 4:

            break;
        default:
            System.out.println("Illegal Operation");

        } // end of Switch

        return choice;
    } // end of getSelection()

    // Value - returning method =======================================
    private static double doTheMath(int choice, double num1, double num2) {
        double result;
        if (choice == 1)
            result = num1 + num2;
        else if (choice == 2)
            result = num1 - num2;
        else if (choice == 3)
            result = num1 * num2;
        else
            result = num1 / num2;

        return result;
    } // end of doTheMath(int choice, double num1, double num2)

    // Void - returning method =======================================

    private static void displayResult(int choice, double result, double num1, double num2) {
        // formatting numbers to 1 decimal places
        DecimalFormat df = new DecimalFormat("#,###,##0.0");

        if (choice == 1)
            System.out.println(num1 + " + " + num2 + " = " + df.format(result));
        else if (choice == 2)
            System.out.println(num1 + " - " + num2 + " = " + df.format(result));
        else if (choice == 3)
            System.out.println(num1 + " * " + num2 + " = " + df.format(result));
        else
            System.out.println(num1 + " / " + num2 + " = " + df.format(result));

    } // end of displayResult(int choice, double result, double num1, double
        // num2)
} // end of main()
/* Output ==========================
Enter the first number: 5

Enter the second number - (Cannot be zero for Divison): 2

1. Addition
2. Subtract
3. Multiply
4. Divide

Which math operation? Enter your choice:  4
5.0 / 2.0 = 2.5
*/
\$\endgroup\$
7
\$\begingroup\$

Very nice job on this Java code. It is very neat and organized.


Comments like this:

// end of getFirstNumber()

and this:

// Value - returning method =======================================

and this:

// Returns First number to main

are completely unnecessary.

I know that that last } is the end of getFirstNumber; just by reading your code, I can see that. I know that that method returns a value; I can see that right beneath this comment. I know that that return statement is returning a number to whatever called it.

However, you were on the right track with the second comment. You should look more into JavaDoc, because in that is where you describe return values and other things about the method.


In your methods getFirstNumber and getSecondNumber, you create a new java.util.Scanner like this:

Scanner input = new Scanner(System.in);

I don't know about you, but that seems a little unnecessary to me to have to create the same exact thing for each method.

Rather than creating this Scanner in these methods, you should set it to a field of this class. Then, rather than creating two Scanners, you are only creating and using one.


Let's look at the method bodies of getFirstNumberand getSecondNumber:

getFirstNumber:

Scanner input = new Scanner(System.in);

System.out.print("Enter the first number: ");
double num1 = input.nextDouble(); // First number storage

return num1; // Returns First number to main

getSecondNumber:

Scanner input = new Scanner(System.in);

System.out.print("\nEnter the second number - "
        + "(Cannot be zero for Divison): ");
double num2 = input.nextDouble(); // Second number storage

return num2;

These two methods look identical, except for the fact that the second method added a 2 to the end of the variable names, and added a different message.

Since these are so similar, you might as well just merge these two methods into a single method; after all, they both return a double input from the user.

Now you may wonder, what am I going to do about telling the user to not input a 0? Well, you already have one problem with this: you don't do any checking if the user actually does input a 0.

So, you can just leave this message out and you send the user an error if they do enter a 0 AND they are attempting to divide by 0.


You have a problem with your design. Here is what happens:

  1. Ask the user for a number.

  2. Ask the user for another number.

  3. Ask the user for an operation

  4. Confirm that the operation exists.

  5. If not, tell the user.

  6. Perform the specified operation on the inputted numbers.

See the problem? It is on step 5: sure, you tell the user that they entered an invalid operation, but you don't do anything about it; you just continue trying to perform the operation as if nothing ever happened.

This is obviously not good. You want the program to do something about it invalid input is entered. In this case, rather than continuing execution of the code, the program should go back to the beginning and prompt the user again.

That can be achieved using a while loop.


You have a construct like this there separate times in your code (pseudo-code):

if choice == 1
    [code]
if choice == 2
    [code]
if choice == 3
    [code]
if choice == 4
    [code]

You wouldn't have to write this three separate times if you used this thing called a enum.

Now, you specified that you are a beginning to Java in your post and in your post's tags, so this may be very confusing. However, if you read the link I just posted, you should be able to follow along.

I have posted an answer about using an enum for a calculator before and I recommend that you read that since I would share the same solution for your code.

\$\endgroup\$
  • \$\begingroup\$ Thank you for this detailed response. It'll take me some time to understand what you wrote. The JavaDoc is interesting, but I'm not sure when and where to add this. Before every variable seems like it'd be messy. As for putting the scanner in the class field. I don't believe I've learned how to do this yet. Also I get a warning about a leak too. As for the user entering a 0 for num2, I thought about this and I couldn't figure out a solution how do to it so I abandoned this, for now. \$\endgroup\$ – Frank Jul 20 '15 at 17:14
  • \$\begingroup\$ As for the while loop. I like this.I would like to add this as well. When I was trying to implement the while loop I got stuck where would be a good place to put it. I am not sure the type of condition I should use. Am I on the right track with while (num2 != 0) ? How would I return this to main to start over if user enters a 0? \$\endgroup\$ – Frank Jul 20 '15 at 17:18
  • \$\begingroup\$ The enum calculator, I don't understand it after quickly looking at it, but I'm going to try to learn it later. Thanks. \$\endgroup\$ – Frank Jul 20 '15 at 17:19
  • \$\begingroup\$ For the choice input var. How would go about adding checks so that if the user inputs anything other than 1,2,3, and 4 it throws an error message and asks the user to restart the program? \$\endgroup\$ – Frank Jul 20 '15 at 17:26
  • \$\begingroup\$ @Frank You asked a lot of question. Give me a few minutes and I'll try to answer them all in these comments. \$\endgroup\$ – SirPython Jul 20 '15 at 19:24

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.