How to avoid "variable might not have been initialized" near try/catch with System.exit

Question

I want to avoid this ugly pattern:

Long foo = null;
Double bar = null;

try {
  foo = Long.parseLong(args[0]);
  bar = Long.parseLong(args[0]);
} catch (NumberFormatException e) {
  System.out.println(USAGE);
  System.exit(1);
}

doSomethingWith(foo, bar);

I know I can move the initialisations and doSomethingWith into the try block, but imagine:

• doSomethingWith is actually several lines of conditional logic, many method calls, statements etc.
• I don't want to be nesting blocks unnecessarily deeply
• I want to avoid accidentally catch unexpected NumberFormatExceptions thrown from doSomethingWith.

I find the = null and above quite ugly, as well as the use of classes instead of primitives. This ugliness makes me suspect there's a better way to write this code.

My first vision is of course

long foo;
double bar;

try {
  foo = Long.parseLong(args[0]);
  bar = Long.parseLong(args[0]);
} catch (NumberFormatException e) {
  System.out.println(USAGE);
  System.exit(1);
}

doSomethingWith(foo, bar);

but we all know Java throws a couple variable might not have been initialized errors when you try to build that.

What's a better design pattern for this kind of problem? "Better" meaning (for me) does all of this

• avoids the weird = null
• avoids unnecessary class wrappers around primitives
• doesn't nest deeply into the try block
• doesn't catch/hide exceptions it wasn't meant to

If this is not possible, or if my concept of "pretty" code is wrong, please convince me and show the best that can currently be used.

Answer 1

The oddity here is that the compiler doesn't know that System.exit(1); will never return. If it knew that, it would be happy.

So all you need to do is give it something that it knows won't let you get from the catch block to after the try/catch. For example:

try {
  foo = Long.parseLong(args[0]);
  bar = Long.parseLong(args[0]);
} catch (NumberFormatException e) {
  System.out.println(USAGE);
  System.exit(1);
  throw new RuntimeError("Make sure the end of the catch block is unreachable");
}

If you need to do this often, you might want to write a helper method, and throw the result of it (which you'll never use). That way you still only have a single line of code for "I want to quit now".

try {
  foo = Long.parseLong(args[0]);
  bar = Long.parseLong(args[0]);
} catch (NumberFormatException e) {
  System.out.println(USAGE);
  throw HelperClass.systemExit(1);
}

...

public class HelperClass {
  public static RuntimeException systemExit(int exitCode) {
    System.exit(1);
    throw new RuntimeException("We won't get here");
  }
}

Another option I've used quite a bit is to define a sort of "User error" exception. You can then catch that at the top level (in main), print any message and possibly display the usage. That way:

• You can unit test user errors (unit testing System.exit is at least more awkward)
• You have centralized handling of "what do I want to do if the user made an error" rather than including System.out.println(USAGE) in multiple places
• You don't run into this definite assignment issue

So your code would then be:

try {
  foo = Long.parseLong(args[0]);
  bar = Long.parseLong(args[0]);
} catch (NumberFormatException e) {
  throw new UserInputException("foo and bar must both be valid integers");
}

Answer 2

I couldn't resist! This is another cleaner (but longer) solution.

// SomeModel(a: long, b: long)
public static Optional<SomeModel> validate(final String[] args) {
  try {
    // Probably some other null checks and things like that here also
    return Optional.of(new SomeModel(Long.parseLong(args[0]), Long.parseLong(args[1])));
  } catch (final NumberFormatException e) {
    System.out.println(USAGE);
    System.exit(1);
  }
  return Optional.empty(); // Or use some other "defaults" for that model
}

public static void doSomethingWith(final SomeModel input) { // ...signature changed!
  // Do your stuff here
}

public static void main(final String[] args) {
  validate(args).ifPresent(it -> doSomethingWith(it));
}

This flow is more clear but it's more verbose. The validations are separated and there is nothing else to do to please the compiler. Having to mode the return might be a drawback, but it pays off later on.