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\$
1
  • \$\begingroup\$ Craftbox, Welcome to Codereview! :) \$\endgroup\$
    – Caridorc
    Aug 14 '15 at 18:14
5
\$\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\$
1
  • \$\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
4
\$\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\$
1
  • 1
    \$\begingroup\$ Thanks, i still have one problem though, in is never closed \$\endgroup\$
    – craftxbox
    Aug 14 '15 at 18:22
4
\$\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\$

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