8
votes
\$\begingroup\$

My friend does a bunch of Java work (commandline, still toying around) and I notice a bunch of try/catch blocks like this:

   try {
       double a = Double.parseDouble(secondInput);
   }
   catch(NumberFormatException nFE) {
       System.out.println("The input was not an integer.");
       try {
           Thread.sleep(1997);
       } catch (InterruptedException e) {
           // TODO Auto-generated catch block
           e.printStackTrace();
       }
       System.exit(0);
   }

Which, IMO doesn't look very pretty. And I'm wondering what the "better" way to write this code would be.

\$\endgroup\$
  • \$\begingroup\$ Please check: stackoverflow.com/questions/5774505/… \$\endgroup\$ – Vicente Plata Apr 25 '11 at 1:59
  • 1
    \$\begingroup\$ There are a number of problems with this code. What element in particular are you asking about - the nested try/catch blocks, like Vicente mentions? \$\endgroup\$ – Michael Petrotta Apr 25 '11 at 2:01
9
votes
\$\begingroup\$

There are a couple of issues.

  • Why is the application pausing for 2 seconds before exiting? Why not just exit straightaway? (And if the answer is that you are running this via Windows 'cmd.exe' then you are better off handling the pausing at that level ...)

  • Given that you do need to pause, then you are better of writing a helper method to do something like "print a message, pause, and exit with a non-zero exit code". Then use that in multiple places, as required.

  • You should be setting a non-zero exit code when you exit due to a failure. This allows the script or whatever that launched the java app to detect the failure and (maybe) do something about it; e.g. pause so that the user can see the error message ...

  • The InterruptedException is not a program error, so printing a stacktrace is probably not appropriate.

Figuring out what to do with an InterruptedException exception is not straightforward. The exception indicates that something (either the user or the application) has interrupted the application, either before or during the sleep.

  • In this use-case you are just about to exit anyway, so the best strategy is to ignore it an exit anyway.

  • You need to decide what is a sensible thing to do. Some possibilities are:

    • Ignore the interrupt and carry on regardless.

    • Take steps to cause the application to stop what it was doing; e.g. throw a custom unchecked "stop now" exception. (You could also call System.exit() directly, but that can also be a bad thing to do, depending on the nature of the code. For instance, calling System.exit() in a library ... or a webapp ... is a big non-no.)

    • Punt: reinstate the "interrupted" state by calling Thread.currentThread().interrupt() in the exception handler. For instance, you might do this if some enclosing code is checking the "interrupted" flag.

\$\endgroup\$
  • \$\begingroup\$ My friend's a bit of a trial/error kind of learner. I'll pass this on to him. I get it that Eclipse isn't too beginner friendly then. \$\endgroup\$ – digitxp Apr 25 '11 at 3:37
  • \$\begingroup\$ @digitxp - these issues are nothing to do with Eclipse. \$\endgroup\$ – Stephen C Apr 25 '11 at 4:40
  • 1
    \$\begingroup\$ @Stephen C: I get the feeling the 1997 milliseconds are the grace period given to the user to read the message. :/ \$\endgroup\$ – Grant Thomas Apr 25 '11 at 15:41
  • \$\begingroup\$ @Mr. Disappointment- The message is printed to the console- it won't disappear on System.exit(); \$\endgroup\$ – Rob Lourens Apr 26 '11 at 0:24
  • 2
    \$\begingroup\$ @Rob Lourens - ... unless you are running it on Windows from a lame "BAT" file that closes the console window on exit. \$\endgroup\$ – Stephen C Apr 26 '11 at 5:58
3
votes
\$\begingroup\$

Looking at your specific case of Thread.sleep, and knowing other I/O exception-throwing methods (close(), for example), how about using helpful libraries like Google Guava?

For the Files example, you can use Closables.closeQuietly(filehandle), which does not throw an exception. See:

For your sleep example, you could make similar methods (if you really wanted a thread to sleep ~2 seconds), Sleeper.sleepQuietly(2000), etc.

\$\endgroup\$
1
vote
\$\begingroup\$

You hit infamous checked exceptions feature in Java. Here is one of the basic articles on this, by Bruce Eckel but you can find more by googling "checked exceptions in Java".

In most cases it's hard to avoid try/catch blocks but all you need to do in most cases is to rethrow the exception without any processing. In my code I am using the following pattern (using Throwables from Google Guava library):

void myMethod() {
  try {
     APIFunction1();
     APIFunction2();
  } catch (Exception e) { throw Throwables.propagate(e); }
}

I consider this the least necessary evil. You can write similar utility yourself, quite easily.

\$\endgroup\$
  • \$\begingroup\$ I forget which language is the "let it crash" one, but I wish Java was. :-/ \$\endgroup\$ – digitxp Apr 25 '11 at 2:31
1
vote
\$\begingroup\$

How about :

try {
    double a = Double.parseDouble(secondInput);
}
catch(NumberFormatException nFE) {
    displayNotIntegerErrorAndExit();
}

[...]
private void displayNotIntegerErrorAndExit() {
   System.out.println("The input was not an integer.");
   sleepFor(1997);
   System.exit(0);
}

private void sleepFor(long delayInMs) {
   try {
       Thread.sleep(delayInMs);
   } catch (InterruptedException e) {
       e.printStackTrace();
   }
}

Note: I did not check if it compiles or anything, but that is the way I do it normally. And it makes it easy to see if you are duplicating code (i.e. if you are making 300 sleepFor() methods).

Edit: As an additional point. When handled correctly, the increase in readability and flexibility or the code that comes from exception far outweighs the minor overhead of the exception handling.

\$\endgroup\$
0
votes
\$\begingroup\$

Not sure on Java since it is a while since I coded with it but in C#, a try-catch block carries a performance overhead that is best limited. A single block is expensive, and best replaced with conditional evaluation to prevent the exception being thrown in the first place, but using nested blocks is okay for basic home projects but if I had to peer review that code I would push it right back to the source developer and ask them to refactor the codefor optimisation.

\$\endgroup\$

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