0
\$\begingroup\$

I'm new to programming and just made a basic calculator on my own.

Can anyone see if there is any room for improvement in the code, like stuff i should do different.

package calculator;

import java.util.Scanner;

public class Main {

static Scanner userInput = new Scanner(System.in);



public static void main(String[] args) {
    float numOne;
    float numTwo;
    float answer = 0;
    String option;
    boolean loop = true;

    while(loop==true){
    System.out.print("Addition, Subtraction, Multiplication or Division? (WRITE ONLY IN LOWERCASE!) \n Type:");
    option = userInput.next();
    if(option.equals("addition")){
        System.out.print("Enter your first number: ");
        numOne = userInput.nextFloat();
        System.out.print("Enter your second number: ");
        numTwo = userInput.nextFloat();
        answer = addition(numOne, numTwo);
        getAnswer(answer);
    }else if(option.equals("subtraction")){
        System.out.print("Enter your first number: ");
        numOne = userInput.nextFloat();
        System.out.print("Enter your second number: ");
        numTwo = userInput.nextFloat();
        answer = subtraction(numOne, numTwo);
        getAnswer(answer);
    }else if(option.equals("multiplication")){
        System.out.print("Enter your first number: ");
        numOne = userInput.nextFloat();
        System.out.print("Enter your second number: ");
        numTwo = userInput.nextFloat();
        answer = multiplication(numOne, numTwo);
        getAnswer(answer);
    }else if(option.equals("division")){
        System.out.print("Enter your first number: ");
        numOne = userInput.nextFloat();
        System.out.print("Enter your second number: ");
        numTwo = userInput.nextFloat();
        answer = division(numOne, numTwo);
        getAnswer(answer);
    }else{
        System.out.println("Error! unvalid option chosen!");
    }
    System.out.println("Do you whish to quit? (if so, do /quit but if not, type /continue)\ntype: ");
    option = userInput.next();

    if(option.equals("/quit")){
        loop = false;
    }

    }



    }

static float addition(float numX, float numY){
    float finalNum;
    finalNum = numX + numY;
    return finalNum;
}

static float subtraction(float numOne, float numTwo){
    float finalNum;
    finalNum = numOne - numTwo;
    return finalNum;
}

static float multiplication(float numOne, float numTwo){
    float finalNum;
    finalNum = numOne * numTwo;
    return finalNum;
}

static float division(float numOne, float numTwo){
    float finalNum;
    finalNum = numOne / numTwo;
    return finalNum;
}   

static void getAnswer(float finalAnswer){
    System.out.println("The answer is " + finalAnswer);
}


}
\$\endgroup\$
1
\$\begingroup\$

You really don't need:

    System.out.print("Enter your first number: ");
    numOne = userInput.nextFloat();
    System.out.print("Enter your second number: ");
    numTwo = userInput.nextFloat();

four times. Just get the function first and then the two numbers independent of which function was chosen.

Also, do you really need to write functions for simple mathematical operations?

Finally, you don't need getAnswer(answer); four times either.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the answer! I wrote methods for it just because i wanted to do something with more then one method, i know it isn't really necessary. \$\endgroup\$ – Svante Jan 16 '16 at 21:18
1
\$\begingroup\$

Biggest suggested change is to pick values separately from operator, which is mentioned by Barry.

Just minor comments:

loop is unneeded, just use while(true)

There is no need for the operator functions (addition, etc), just use the actual operator. This simplification would remove 24 lines of code without adding any complexity/clarity issues. If you do decide to keep them, rewrite them to just return a + b;, return a * b; etc. instead of initializing a float, then filling in the float with the result, and then returning the float.

Also, fix your indentation/spacing:

while(true) {
    if (condition) {
        //code
    }
}
\$\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.