Skip to main content
Code Review

Return to Answer

edit because of comments
Source Link
bazola
bazola
  • 8.6k
  • 2
  • 34
  • 73

EDIT As noted in the comments, in this case it is better to use a constant instead of StringBuilder. However if you are ever building a string inside of a loop, you will definitely want to be familiar with StringBuilder because in that case it really is much better.

As for the constant, you have a couple of options. At the very least you could make this a constant rather than typing it out twice:

String DIVIDING_LINE = "=====================================";

Note that constants in Java are typically declared in all caps. Or you could make the whole thing a constant like this:

    String MENU = "=====================================\n"
                + "           Calculator V1.0\n"
                + "=====================================\n"
                + "1. Addition\n"
                + "2. Subtraction\n";

That way if you have to create the menu again, you don't have to build the whole string again, you can just use the constant.

EDIT As noted in the comments, in this case it is better to use a constant instead of StringBuilder. However if you are ever building a string inside of a loop, you will definitely want to be familiar with StringBuilder because in that case it really is much better.

As for the constant, you have a couple of options. At the very least you could make this a constant rather than typing it out twice:

String DIVIDING_LINE = "=====================================";

Note that constants in Java are typically declared in all caps. Or you could make the whole thing a constant like this:

    String MENU = "=====================================\n"
                + "           Calculator V1.0\n"
                + "=====================================\n"
                + "1. Addition\n"
                + "2. Subtraction\n";

That way if you have to create the menu again, you don't have to build the whole string again, you can just use the constant.

Source Link
bazola
bazola
  • 8.6k
  • 2
  • 34
  • 73

Comments

Since this is a beginner program you can be excused for the very bad comments, but you still need to hear about them.

Comments like this:

    // 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

Are all pretty much useless. Your comments should typically explain Why you have done something, and almost never explain What your code is doing.

It is completely obvious that a new scanner object is being created, and that the while loop will keep the program running until the user exits.

On the other hand, the comment that you have for fnum, snum, and answer is somewhat helpful. Unfortunately it is helpful for the wrong reasons.

Bad Variable Names

These are bad variable names:

int fnum = 0;
int snum = 0;

You should make a strong effort to make your variable names as descriptive as possible. In this case I would recommend simply calling the variables firstNumber and secondNumber. Using abbreviations like this is almost never worth the characters that you will save.

StringBuilder

Repeatedly calling System.out.println is very resource intensive. It doesn't matter very much for this program, but it could matter a lot when you are working on something more complicated later on. On the other hand, StringBuilder is very efficient.

So this:

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): ");

Could become this:

StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append("=====================================");
stringBuilder.append("\n");
stringBuilder.append("           Calculator V1.0         ");
stringBuilder.append("\n");
stringBuilder.append("=====================================");
stringBuilder.append("\n");
System.out.println(stringBuilder.toString());

It's more lines of code but it is much more performant.