5
\$\begingroup\$

I made this simple addition calculator in Java and I'm wondering how I can improve it.

package com.craftxbox.main;

import java.util.Scanner;

public class Main {
    public static void main(String[] args){
        int inInt1;
        int inInt2;
        int outInt;
        @SuppressWarnings("resource")
        Scanner in = new Scanner(System.in);
        inInt1 = in.nextInt();
        inInt2 = in.nextInt();
        outInt = inInt1 + inInt2;
        System.out.println(outInt);

    }
}
\$\endgroup\$
  • \$\begingroup\$ Craftbox, Welcome to Codereview! :) \$\endgroup\$ – Caridorc Aug 14 '15 at 18:14
4
\$\begingroup\$

You have written some good code, but what if the user does not enter a number, but makes a mistakes? You can, and should, catch that error. What about this:

import java.util.InputMismatchException;
import java.util.Scanner;

public class Calculator
{
    static Scanner scanner;

    public static void main(String args[])
    {
       try
       {
           scanner = new Scanner(System.in);
           double firstNum, secondNum, answer;

           System.out.println("Enter your first number: ");
           firstNum = scanner.nextDouble();

           System.out.println("Enter your second number: ");
           secondNum = scanner.nextDouble();

           answer = firstNum + secondNum;
           System.out.println(answer);
       }
       catch (InputMismatchException e)
       {
           System.out.println("Invalid input");
       }
       finally
       {
           scanner.close();
       }

}

}

I think a try - catch - finally is a great way of getting user input. This way your problem of the Scanner not being closed, is solved. If this is a little over you head right now, don't worry. It does what it says: we "try" the code in the try block, but if something goes wrong, in this case a "Inputmismatchexception", which means if the user inputs something wrong, we catch it and we display a message saying something went wrong.

Note that I have also changed the variable names. By reading the name, you should immediately know what it is and so names like firstNum and answer are a little better than inInt1 and stuff.

Hope you're still with me. If something is not clear, please feel free to ask some more questions. I myself am a beginner to Java and programming in general and this is the first question I answered.

\$\endgroup\$
  • \$\begingroup\$ Good answer, remember to: ¤ Declare and assign at the same time: double firstNum = Scanner ... ¤ Reduce the try block as much as possible \$\endgroup\$ – Caridorc Aug 15 '15 at 7:09
2
\$\begingroup\$

Declare and assign at the same time to avoid code-duplication:

    int inInt1 = in.nextInt();
    int inInt2 = in.nextInt();
    int outInt = inInt1 + inInt2;

Delete the three lines of declaration.

In fact, the whole program can be reduced to:

import java.util.Scanner;

public class Main {
     public static void main(String[] args){
           Scanner in = new Scanner(System.in);
           System.out.println(  in.nextInt() + in.nextInt() );
     }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks, i still have one problem though, in is never closed \$\endgroup\$ – craftxbox Aug 14 '15 at 18:22
2
\$\begingroup\$

Since Java 7, the recommended way of closing Closeable resources such as Scanner is to use try-with-resources:

try (Scanner scanner = new Scanner(System.in)) {
    // ...
}
// no need for explicit scanner.close() here, 
// since the variable is scoped within the try statement and will be closed at the end

As mentioned in @Maxim's answer, you should also consider validating the inputs.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.