patternjavaMinor
Cleaning up calculator program
Viewed 0 times
programcleaningcalculator
Problem
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;
double fnum, snum, answer;
System.out.println("Calculator Program Initialized");
System.out.println("Please select your math operator");
System.out.println("Enter 1 for Addition");
System.out.println("Enter 2 for Subraction");
selection = checker.nextInt();
if (selection == 1){
System.out.println("Enter your first number here");
fnum = checker.nextDouble();
System.out.println("Enter your second number here");
snum = checker.nextDouble();
answer = fnum+snum;
System.out.println("Answer: " + answer);
System.out.println("Thank you for using my calculator");
}else{
System.out.println("Enter your first number here");
fnum = checker.nextDouble();
System.out.println("Enter your second number here");
snum = checker.nextDouble();
answer = fnum-snum;
System.out.println("Answer: " + answer);
System.out.println("Thank you for using my calculator");
}
}
}Solution
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("Please select your math operator");
System.out.println("Enter 1 for Addition");
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;
default: answer = Double.NaN;
}
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.)Code Snippets
class apples{public static void main(String args[]){Scanner checker = new Scanner(System.in);try(Scanner in = new Scanner(System.in)){System.out.println("Calculator Program Initialized");
System.out.println("Please select your math operator");
System.out.println("Enter 1 for Addition");
System.out.println("Enter 2 for Subraction");Context
StackExchange Code Review Q#37598, answer score: 5
Revisions (0)
No revisions yet.