Nice solution. This code is pretty legible, which is great if you have to debug anything. There are a few simple ways you can make your program more user-friendly and resistant to user error.
Input:
For the user, it can be tedious to type in their values on every line, always being asked for
Num:
Operator:
Num:
Operator:
...
Can you use in.nextDouble()
and in.next(".")
to read in all of your numbers and operators at once, from a line like 10+9-7
or 10+9-7=
? in.next(String pattern)
matches a regular expression, and .
is the pattern for any single character.
Error Handling:
What happens if the user types in a bad operator? If someone tries to ask for 11+10_9
, the operation is ignored in this case. Instead of getting the result of 11+10-9
= 12, they could get a result of 21 without realizing there was a mistake. A good way to protect against that is to use a default:
statement in your switch block. A way to handle these errors and let the program or the user know there was a mistake is by writing
default: throw new IllegalArgumentException(equ + " is not a valid operator");
Object Oriented Programming:
Generally, static variables should be reserved for variables that are shared between multiple instances of a class, and any other variables should be made local or non-static. The static variables you've defined here work properly, but reorganizing the methods and changing the variables into local ones will make it easier to add functionality to this calculator later.
public static double answer;
Rather than saving the answer in a static or object-level variable, you can return it from your functions. This lets you keep more careful track of what makes changes to it. Imagine if you had a lot of different functions like calculate
that incrementally update the answer every time they execute. If you started seeing the wrong answer due to a bug in the code, it could be difficult to trace it to the broken function.
To handle answer
the object-oriented way, rename main()
to whatever you like, change both of your functions from void
to double
type, and return the partial answer and the end answer from each of them, handling them locally. You can make a new main()
that calls your renamed function and handle the printing of the answer in main()
instead of the function that does the work.
public static boolean done = false;
done
is never actually changed or used. while(true)
is functionally equivalent to the loop that you wrote. This is a fairly common pattern, and there are a few ways to go about organizing it. Yours is handy because the variable done
communicates that this loop will continue until something is 'done'. The best place to explain to the reader how a while()
loop is supposed to terminate is right there in the condition.
One more descriptive (but also less clean) way to write that condition would be
String operand;
while((String operand = in.nextLine()) != '='){
This assigns the operator to operand
and performs the comparison without the need for a break
statement. You can use a do-while loop to handle the first number and print "Operator: " to the console before reading operand
, if you like.
public static Scanner in = new Scanner(System.in);
This is often tolerated as a static variable, since we're very unlikely to change the fact that we're reading from System.in in the middle of our calculations. On the other hand, in
doesn't need to be static, and since it's a resource that should be closed when we're done with it, the best practice is to make it a local variable or manage it carefully as needed. The object-oriented approach is to use a loosely-coupled function like readExpression()
for your input, and then let all of the calculation happen in a different function.
You might also want to be consistent with your arguments in
public static void calculate(double num1, String equ, Double num2){
double
is a primitive type, while Double
is a class. That distinction won't cause a logical error here, but consistency always helps when a program grows larger and more complex.
Order of Operations:
Your program handles each step of the calculation correctly, but it doesn't understand Order of Operations. That's a more complex concept that you'll have to look at a whole expression for instead of being able to handle each operation piece by piece.
The fully object-oriented solution is to treat the expression and the individual operations as their own objects and apply logic to those objects piece by piece. You could, for instance, create a list of numbers and operations, and scan through that list, applying the highest-priority (multiplication and division in this case) operations first.
The fully functional solution is to scan the expression and use recursion to decide which numbers need to into calculate()
first, returning the results of each calculate()
step up until the base function returns the final answer.
There are many ways to do it, and you can think about how you'd like to implement it in your calculator