Re: Exception Handling
Lew <noone@lewscanon.com> wrote in news:jjh39f$crc$1@news.albasani.net:
Novice wrote:
snip
I have a utility class called LocalizationUtils which basically
houses convenience methods dealing with i18n/l10n. One of its methods
is getResources(). It expects two parameters, a String representing
the "base name" (the leading part of the resource file name) and a
locale.
Here's the code for getResources() with all comments and error
handling (aside from the empty catch block) stripped out:
====================================================================
static public ResourceBundle getResources(String baseName, Locale
locale {
ResourceBundle locList = null;
Don't use throwaway initializations, usually.
Why?
If I omit that line and simply use the following in the try block:
ResourceBundle locList = ResourceBundle.getBundle(baseName, locale);
locList isn't visible after the try/catch so that I can return it.
try {
locList = ResourceBundle.getBundle(baseName, locale);
}
catch (MissingResourceException mrExcp) {
Log and return 'null' if that's the recovery strategy for this
exception.
Is that what I should do, return null if there is a problem getting the
resource?
I'm not sure if I should be returning anything at all. Maybe I should
just log and stop the program?
}
return (locList);
}
====================================================================
The program which is executing is Foo. It has a method called
getLocalizedText() which is the one executing
LocalizationUtils.getResources(). The value of the baseName is a
constant coded in an interface of constants called FooConstants which
is implemented by Foo. Here's the code for getLocalizedText() without
any error handling and just one relevant comment:
Using an interface to define constants is the "Constant Interface
Antipattern". (Google it.) Use a class to define constants, not an
interface.
An interface is there to define a type. Using it merely to define
constants violates the spirit of interfaces, which is to avoid
implementation.
Fair enough. I've converted each constants interface to a class with a
private constructor that throws an exception if someone tries to
instantiate it.
====================================================================
private void getLocalizedText() {
Locale locale = Locale.getDefault();
this.myText = LocalizationUtils.getResources(TEXT_BASE, locale);
this.myMsg = LocalizationUtils.getResources(MSG_BASE, locale);
/* Work with the localized resources. */
}
====================================================================
In terms of trouble spots, I know from experience that getResources()
will throw a MissingResourceException, which is an unchecked
exception, if the base name is misspelled. A null value in either
parameter is also going to make the getResources() method fail:
ResourceBundle.getBundle() will throw a NullPointerException if
either parameter is null.
My objective is to code getResources() and getLocalizedText() as
professionally as possible. If the methods fail, I want to write a
clear message to my log and the log record needs to include a
stacktrace. (I'm
example:
logger.error(message, exception);
Exactly the kind of thing I had in mind.
assuming that a console is not necessarily available to the operators
running this program and I need to know where the error took place.
Based on an article Arved suggested in another thread,
http://www.javacodegeeks.com/2011/01/10-tips-proper-application-
logging.html, I'm inclined to minimize "position" information - class
name, method name, line number - in the log and instead get all of
that from the stacktrace.)
I disagree.
I'd keep that class/method info in the log message. Line information
is less valuable unless your methods are far, far too long.
Tip 6 in the article makes the point that it is redundant and expensive
to get class, method and line information if it is already in the log
message. A stacktrace will tell me all of that and more so shouldn't I be
concentrating on getting a stacktrace, plus whatever I'll need that isn't
in the stacktrace, like the date and time of the problem?
Now, finally, here are some questions:
Since the only exceptions I anticipate, MissingResourceException and
NullPointerException, are unchecked exceptions, I gather than I
shouldn't really do anything about them aside from logging when they
happen. Okay,
I disagree. You should end the program gracefully and get someone to
fix the programming error right away.
But what's the right way to end the program? System.exit()? That seems
the obvious way but as I've said further down, that doesn't seem to be
the method used in the books and articles I've been reading. What's the
better way?
fair enough; I certainly don't want to display the source code to my
user, make them enter the correct value for the baseName, make them
recompile the program and then run it again! But when/where should I
log the error? For instance, if I mess up my coding somehow and
inadvertently
In the log file.
Yes, I realize that it goes in the log file. I mean when and where in my
code do I do that? In getResource() or getLocalizedText()?
pass a null in the baseName, should getResources() be testing its
input parameters individually to see if they are null and write a
message to the log if they are null from within that method? I'm
inclined to say yes
Always check all parameters for validity, either by an explicit check
for a public or protected method, or by controlling what's passed to
package-private or private methods.
A normal practice is to have an application-specific checked exception
to throw, or to throw the standard unchecked exception. For example:
if (argument == null)
{
final String msg = "null argument";
IllegalArgumentException except = new
IllegalArgumentException(msg); logger.error(msg, except);
throw except;
}
or similarly for application checked exception 'FooException'.
throw new FooException(new IllegalArgumentException(msg));
At least one of the articles/books recommended against creating custom
exceptions unless necessary, preferring to use existing exceptions where
possible.
The Java Tutorial puts it this way:
"You should write your own exception classes if you answer yes to any of
the following questions; otherwise, you can probably use someone else's.
- Do you need an exception type that isn't represented by those in the
Java platform?
- Would it help users if they could differentiate your exceptions from
those thrown by classes written by other vendors?
- Does your code throw more than one related exception?
- If you use someone else's exceptions, will users have access to those
exceptions? A similar question is, should your package be independent and
self-contained?"
I don't _think_ I qualify to create my own exception under any of these
conditions so I'm inclined to stay with the standard ones. Or am I
missing something?
Assuming I'm not, I'm inclined to use the first approach you suggested.
Or perhaps use the technique but throw a NullPointerException. Bloch
implied that IllegalArgumentException was fine for bad values but that
NullPointerException was preferred where a parameter had a null value.
Somewhere up the stack you should catch the exception and convert it
to valid program state"
catch(RuntimeException except)
{
forwardProgramControlToErrorScreen();
}
I went into Eclipse for a minute and amended getResources() to look like
this:
========================================================================
static public ResourceBundle getResources(String baseName, Locale locale)
throws NullPointerException, IllegalArgumentException {
if (baseName == null) {
final String msg = "The base name cannot be null.";
NullPointerException nullPointerException = new NullPointerException
(msg);
Logger logger = Logger.getLogger(CLASS_NAME);
logger.log(Level.SEVERE, msg, nullPointerException);
throw nullPointerException;
}
if (locale == null) {
final String msg = "The locale cannot be null."; //$NON-NLS-1$
NullPointerException nullPointerException = new NullPointerException
(msg);
Logger logger = Logger.getLogger(CLASS_NAME);
logger.log(Level.SEVERE, msg, nullPointerException);
throw nullPointerException;
}
//ResourceBundle resourceFile = null;
try {
ResourceBundle resourceFile = ResourceBundle.getBundle(baseName,
locale);
return(resourceFile);
}
catch (MissingResourceException mrExcp) {
String msg = "Unable to find resources for base name, " + baseName + ",
and locale, " + locale + ". Check the spelling of the base name.";
throw new IllegalArgumentException(msg);
}
}
========================================================================
That works just fine and my exception, with stacktrace, is logged before
I've left getResources(). But why am I throwing the exception at the end
of the if (baseName == null) block? Why not just exit the program
gracefully instead of throwing the exception? If I pass it back up to the
caller, what is is actually supposed to do, given that the message has
been logged and recovery is not practical? Am I only passing it up so
that getLocalizedText() can stop the program?
to that but I'm not sure what to do next. It seems pointless to carry
on with getResources() since ResourceBundle.getBundle(baseName,
locale) will fail on a NullPointerException with a null in either
parameter. I could throw a NullPointerException so that
getLocalizedText() can deal with it. But how do I get a stacktrace
into the log if I follow that strategy? It's easy enough to get the
stacktrace once I've caught an exception but I'm just talking about
recognizing that an input parameter is null; there's no exception at
that point to put in my log.
There is if you create one. That's why Java has the 'new' operator.
Yes, I see that from your technique of creating one. I hadn't anticipated
that approach.
If I let getResources() throw NullPointerException if either
parameter is
Better, use 'IllegalArgumentException'.
null, then I can let getLocalizedText() catch those
NullPointerExceptions and get the stacktraces from the exception and
write them to the log. In that case, I may as well just check the
input parameters for nulls, and simply throw NullPointerException
with a specific message within
'IllegalArgumentException'.
I have no strong feelings about this one way or the other but Bloch, on
page 248 of Effective Java (2nd edition) says: "Arguably, all erroneous
method invocations boil down to an illegal argument or an illegal state,
but other exceptions are standardly used for certain kinds of illegal
arguments and states. If a caller passes null in some parameter for which
null values are prohibited, convention dictates that NullPointerException
be thrown rather than IllegalArgumentException."
Can we agree that whether I use IllegalArgumentException or
NullPointerException in the situation I'm describing in my own code is
simply a matter of personal style or preference? Or is there a reason to
prefer IllegalArgumentException that Bloch failed to consider?
Like I said, I'm fine with either approach.
getResources(); then getLocalizedText() can have a try/catch block.
The try/catch can detect the NullPointerException and write both the
message and the stacktrace to the log.
Write the log at the point where you detect the error, not only
further up the stack.
That's what I wanted to do. I just wasn't sure how to get the stacktrace
into the log until you showed me that technique ;-)
But I gather that you should handle the error as close to where it
happened as possible when you can. I'd tend to prefer to handle the
null parameter values in getResources() except that I'm not sure how
to write the stack trace to the log before any exception has
happened.
IllegalArgumentException exception = new
IllegalArgumentException(msg); logger.error("Unable to proceed",
exception);
One other questions. When, if ever, should I execute System.exit()
with a non-zero integer? In my example, I know that program Foo can't
proceed
Never. Program exit should be under user control.
So, given that my application has a GUI, leave the application suspended
at the exception and make the user click the Close button? Why is that
better than just exiting given that the program can't proceed without the
missing resources in this case?
If this were a batch program rather than one with a GUI, would we close
with System.exit()? Or wait for the operator to notice the program is
suspended and make him kill it?
without the resources it is trying to get in getResources() so if I
simply throw exceptions and write to the log and then let the program
keep going, I'm simply going to get another NullPointerException on
the first statement that tries to use the resources. I'm thinking
that I should do a System.exit(1) after I've logged a null parameter
or in the event of a MissingResourceException in getResources(). But
I have yet to see System.exit() used in ANY example of error handling
in any resource I've looked at so far.
Hmm.
--
Novice