patternjavaModerate
Simple beginner Java calculator
Viewed 0 times
beginnersimplecalculatorjava
Problem
This is my first calculator in Java. I just started learning a week ago so any feedback (positive and negative) is welcome. Tell me what's good and what I can improve on.
```
import java.util.Scanner;
public class Calculator {
public static void main(String[] args) {
// Creates scanner object
Scanner scanner = new Scanner(System.in);
// Controls the start and stop of the program
boolean run = true;
// Sets and resets the operand and answer variables
int fnum = 0;
int snum = 0;
int answer = 0;
// While loop that keeps the program running until user exits
while (run) {
// Calculator interface
System.out.println("=====================================");
System.out.println(" Calculator V1.0 ");
System.out.println("=====================================");
System.out.println("1. Addition");
System.out.println("2. Subtraction");
System.out.println("3. Multiplication");
System.out.println("4. Division");
System.out.println("5. Exit");
System.out.printf("Please pick an operation(1-5): ");
// Prompts user to choose an operation
int operation = scanner.nextInt();
// Does stuff depending on what integer the user enters
switch (operation) {
case 1:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum + snum;
System.out.println("The answer is: " + answer);
break;
case 2:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
```
import java.util.Scanner;
public class Calculator {
public static void main(String[] args) {
// Creates scanner object
Scanner scanner = new Scanner(System.in);
// Controls the start and stop of the program
boolean run = true;
// Sets and resets the operand and answer variables
int fnum = 0;
int snum = 0;
int answer = 0;
// While loop that keeps the program running until user exits
while (run) {
// Calculator interface
System.out.println("=====================================");
System.out.println(" Calculator V1.0 ");
System.out.println("=====================================");
System.out.println("1. Addition");
System.out.println("2. Subtraction");
System.out.println("3. Multiplication");
System.out.println("4. Division");
System.out.println("5. Exit");
System.out.printf("Please pick an operation(1-5): ");
// Prompts user to choose an operation
int operation = scanner.nextInt();
// Does stuff depending on what integer the user enters
switch (operation) {
case 1:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum + snum;
System.out.println("The answer is: " + answer);
break;
case 2:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
Solution
Comments
That is not a good comment. A comment should say why the code does something if it is not clear and the code should say what it does (if the code is unclear, you can use a comment, but this is also a signal you should write clearer code), this comment is just plain noise.
If you are using the comment to remember what the code does, not just because some teacher/book said to, a comment here is perfectly fine:
Duplicated code
Anything catch your eye there? Lots and lots of copy/pasted code. Most of that can be moved to before or after the switch:
Output
This really is a nitpick for now, but this:
Will display the same as this:
Methods
This is a pretty straightforward program, but you should start separating unique components of your program and creating more methods. For example, you could create a method for input, a method for calculating the answer, and a method for output.
Enums
Another thing you should look into is using an
Bug
Your division operation has a bug:
What happens when the user enters
// Does stuff depending on what integer the user entersThat is not a good comment. A comment should say why the code does something if it is not clear and the code should say what it does (if the code is unclear, you can use a comment, but this is also a signal you should write clearer code), this comment is just plain noise.
If you are using the comment to remember what the code does, not just because some teacher/book said to, a comment here is perfectly fine:
// Choose action based on inputDuplicated code
case 1:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum + snum;
System.out.println("The answer is: " + answer);
break;
case 2:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum - snum;
System.out.println("The answer is: " + answer);
break;Anything catch your eye there? Lots and lots of copy/pasted code. Most of that can be moved to before or after the switch:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
switch (operation) {
case 1:
answer = fnum + snum;
break;
case 2:
answer = fnum - snum;
break;
}
System.out.println("The answer is: " + answer);Output
This really is a nitpick for now, but this:
System.out.println(" Calculator V1.0 ");Will display the same as this:
System.out.println(" Calculator V1.0");Methods
This is a pretty straightforward program, but you should start separating unique components of your program and creating more methods. For example, you could create a method for input, a method for calculating the answer, and a method for output.
Enums
Another thing you should look into is using an
enum to designate the selected operation. Using case Operation.Addition: is clearer than case 1:, and there is a very limited set of data that allowed, which is demonstrated and enforced by the enum, which only accepts the allowed set, and is not enforced by an int.Bug
Your division operation has a bug:
snum = scanner.nextInt();
answer = fnum / snum;What happens when the user enters
0? Your program will crash. You must always test for division by 0 in cases like this:snum = scanner.nextInt();
if (snum == 0) {
System.out.println("Invalid input - cannot divide by 0.");
break;
}
answer = fnum / snum;Code Snippets
// Does stuff depending on what integer the user enters// Choose action based on inputcase 1:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum + snum;
System.out.println("The answer is: " + answer);
break;
case 2:
System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
answer = fnum - snum;
System.out.println("The answer is: " + answer);
break;System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();
switch (operation) {
case 1:
answer = fnum + snum;
break;
case 2:
answer = fnum - snum;
break;
}
System.out.println("The answer is: " + answer);System.out.println(" Calculator V1.0 ");Context
StackExchange Code Review Q#98391, answer score: 14
Revisions (0)
No revisions yet.