# Cleaning up calculator program

I recently started learning and programming Java based on a series of beginner videos created by a Youtuber (TheNewBoston), who had a tutorial on a calculator. So based on everything else I've previously learned, I have tried to mix it in to create a bit more advanced one. If you are able to minimize and clean up the code, please give a brief explanation. I'd like some tips as well.

import java.util.Scanner;

class apples{
public static void main(String args[]){
Scanner checker = new Scanner(System.in);
int selection = 0;

System.out.println("Calculator Program Initialized");
System.out.println("Enter 2 for Subraction");

selection = checker.nextInt();

if (selection == 1){
fnum = checker.nextDouble();
snum = checker.nextDouble();
System.out.println("Thank you for using my calculator");
}else{
fnum = checker.nextDouble();
snum = checker.nextDouble();
System.out.println("Thank you for using my calculator");
}
}
}

• Oh, and by the way would I be able to apply a Switch Statement in this, since i tested the variable selection for the operator. Commented Dec 17, 2013 at 16:00
• What if I enter a 0? You should handle that case, too! Commented Dec 17, 2013 at 16:34
• Right, that's what i was thinking =3 Commented Dec 17, 2013 at 16:53
• There are simple Swing calculator and Awt calculator using java Swing Calculator and Awt calculator Commented Jan 2, 2014 at 18:53

class apples{


Naming your class something like apples is always wrong. Java class names start with a Capital letter and are written in CamelCase. Thus, regardless of whether the name itself is good, it should be spelled Apples, not apples.

Also, you need to add a javadoc comment to the class that explains exactly what it does, when to or not use it, and all that.

About the name itself, now. Is Apples really a good summary of what this class is? Looking at your dialogue, probably not; maybe BasicInteractiveCommandlineCalculator would be a better fit? In any case, change the name to something that actually has to do with the class's purpose.

public static void main(String args[]){


Again, no javadoc. Javadoc for a main method should include information about what arguments it takes or how to invoke it. Pretend you were calling it from the command line, exactly what arguments are you passing it? If it doesn't take any arguments, say no args, but at least say something.

Scanner checker = new Scanner(System.in);


Here, checker isn't exactly a great name for this. Call it something shorter, like in.

While your initialization of the scanner is fine, it really ought to be created in a try-with-resources, so you can easily change to use a different input stream as needed, like a file. Instead write:

try(Scanner in = new Scanner(System.in)){


System.out.println("Calculator Program Initialized");
System.out.println("Enter 2 for Subraction");


Doesn't this seem a little wordy for a simple calculator app? You should try talking less, and just say what you mean.

Also, you shouldn't use System.out for permanent output statements. Instead, add another declaration to your try statement. That way, you can easily change the desired stream later, with only minimal modification:

try(Scanner in = new Scanner(System.in); PrintStream out = System.out){


Now, replace the dialog with a much shorter one:

out.println("Enter operation (+ or -):");


Match the input then with in.next(String regex), so it will just wait until there is a valid input.

Handling the input should be way easier, too. Using java 7, we can switch on a string value:

String op = in.next("[+-]");

// ...

switch(op){
case "+": // ...
case "-": // ...
}


If you don't have java 7, then you should still allow the user to input this way, you just need to use if statements instead.

Now, the main problem, though, is in the way it handles the following dialog. Not only is some of the dialog itself duplicated, confusing data with logic, but in the way it computes the answer.

Refactoring the dialog out, and simplifying it, we get much, much neater code:

out.println("x=");
fnum = in.nextDouble();
out.println("y=");
snum = in.nextDouble();

switch(op){
case "+": answer = fnum + snum; break;
case "-": answer = fnum - snum; break;
}

out.println("x" + op + "y=" + result);


This here is only the most basic level of revisions that ought to be made to the code. An application or command that attempts to do this would be expected to support all standard mathematical operations, and to be able to parse a complete expression passed to it as a string.

I once wrote a program like that, that supported all java operators and every function from java.util.Math. My approach was to use a runtime compiler janino to compile as java source code an expression given to it. (It probably would have been simpler to just use syntax trees though; configuring everything was a nightmare.)

• Thank you very much AJ, I appreciate all your advice and corrections. But once again, I'm new to Java so I'll need some time :) Commented Dec 17, 2013 at 16:56

In this case we'd eliminate the repeated code and have left only the different code from your original:

import java.util.Scanner;

class apples{
public static void main(String args[]){
Scanner checker = new Scanner(System.in);
int selection = 1;

while(selection != 0){

System.out.println("Calculator Program Initialized");
System.out.println("Enter 2 for Subraction");
System.out.println("Enter 0 to exit");

selection = checker.nextInt();
if(selection==0) break;

fnum = checker.nextDouble();
snum = checker.nextDouble();

switch(selection){
case 1:
case 2:
break;
default:
System.out.println("Invalid option, try again ");
}

}
System.out.println("Thank you for using my calculator");

}
}


The enter first and second number are repeated, and the print the answer, so we take that out of the condition and differentiate only the operation.

• Even though this simplifies the code, please add a description outside the code block about what you are doing and why you are doing it and why it is better. We don't like "code only" answers here. Commented Dec 17, 2013 at 16:34
• Thank you very much sir, I'll look into this. And technically if I used a switch statement such as while(selection <= 2) that would work too right? Commented Dec 17, 2013 at 16:38
• edited it with a loop Commented Dec 17, 2013 at 16:49
• Yep, that makes more sense now. Thank your fernando Commented Dec 17, 2013 at 16:51