HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Simple calculator in Java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplecalculatorjava

Problem

I've been learning Java for about 3 weeks now, and I was hoping someone could check over my code for me, and let me know how to improve.

I am aware that the maths class could be removed (I could've typed answer = inputA * inputB etc. within the switch statement) but this is just because I wanted to practice using multiple classes.

import java.util.Scanner;

public class Calculator {

public static void main(String[] args) {

    Scanner input = new Scanner(System.in);

    Maths Maths = new Maths();

    double answer = 0;
    double inputA, inputB;
    char operator;
    boolean done = false;

     while (done == false) {
        System.out.print("Please enter your sum: ");

        inputA = input.nextDouble();
        operator = input.next().charAt(0);
        inputB = input.nextDouble();        

        switch (operator) {
            case '+': answer = Maths.add(inputA, inputB);
                      break;
            case '-': answer = Maths.subtract(inputA, inputB);
                      break;
            case '*': answer = Maths.multiply(inputA, inputB);
                      break;
            case '/': answer = Maths.divide(inputA, inputB);
                      break;
            case '^': answer = Maths.power(inputA, inputB);
                      break;
        }

            System.out.println(answer);             
    }       

    input.close();

  }

}


And my second class...

```
public class Maths {

double add(double a, double b) {
double answer = a+b;
return answer;
}

double subtract(double a, double b) {
double answer = a-b;
return answer;
}

double multiply(double a, double b) {
double answer = a*b;
return answer;
}

double divide(double a, double b) {
double answer = a/b;
return answer;
}

double power(double a, double b){
double answer =a;

for (int x=2; x<=b; x++){

Solution

-
power should probably be using Math.pow(). I'm picturing how slow your code would be at calculating 1.00000015000000. Or how inaccurate it'll be when someone wants to raise to a non-integer power. If those are not intended to be allowed, then passing doubles all over the place seems odd. But it'd be better to check beforehand and throw an error than return a blatantly incorrect result, in my opinion.

-
You never set done to true, and thus have an infinite loop; take out the flag and say while (true) or for (;;), so that that's apparent. Don't add the flag back til you're also adding the code that sets it.

-
The fact that you use objects doesn't make your code object-oriented. How you use them is what makes the difference. In this case, you're using classes as namespaces for functions, which is not very object-oriented at all.

In your case, Maths is entirely useless; everything it does would be better done in your own code. This is an example of gratuitous use of objects. If you wanted to practice your OOP, you might want to think about how you'd get rid of that switch statement. Hint:

public interface BinaryOperation {
    public double resultFor(double left, double right);
}

class Multiplication implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left * right;
    }
}

class Addition implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left + right;
    }
}

private Map operations;

...

operations.put('*', new Multiplication());
operations.put('+', new Addition());

...

BinaryOperation op = operations.get(operator);
if (op != null) {
    answer = op.resultFor(inputA, inputB);
}
else {
    System.out.println("Error: Unknown operator");
]

Code Snippets

public interface BinaryOperation {
    public double resultFor(double left, double right);
}


class Multiplication implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left * right;
    }
}

class Addition implements BinaryOperation {
    public double resultFor(double left, double right) {
        return left + right;
    }
}

private Map<Character, BinaryOperation> operations;

...

operations.put('*', new Multiplication());
operations.put('+', new Addition());

...

BinaryOperation op = operations.get(operator);
if (op != null) {
    answer = op.resultFor(inputA, inputB);
}
else {
    System.out.println("Error: Unknown operator");
]

Context

StackExchange Code Review Q#30950, answer score: 17

Revisions (0)

No revisions yet.