Exception Handling


N

Novice

I've been trying to get my head around exception handling for the last
few days concurrently with my efforts to start doing logging correctly.

I've re-read the Java Tutorials section on exception handling. I read
Bloch's remarks on Exception Handling in Effective Java (2nd edition)
several days ago. I'm working my way through Stelting's Robust Java.

It's time to ask some questions to make sure I'm on the right track.

I'm struggling with the best way to revise some of my existing code.
Let's consider a concrete example and then see if we can generalize to
come up with a proper error handling strategy.

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;

try {
locList = ResourceBundle.getBundle(baseName, locale);
}
catch (MissingResourceException mrExcp) {
}

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:

====================================================================
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
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.)

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,
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
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
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.

If I let getResources() throw NullPointerException if either parameter is
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
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.

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.

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
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.
 
Ad

Advertisements

L

Lew

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.
try {
locList = ResourceBundle.getBundle(baseName, locale);
}
catch (MissingResourceException mrExcp) {

Log and return 'null' if that's the recovery strategy for this exception.
}

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.
====================================================================
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);
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.
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.
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.
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));

Somewhere up the stack you should catch the exception and convert it to valid
program state"

catch(RuntimeException except)
{
forwardProgramControlToErrorScreen();
}
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.
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'.

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.
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.
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.
 
A

Arved Sandstrom

Novice wrote:
[ SNIP ]
example:

logger.error(message, exception);


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.

We may be talking/thinking at cross-purposes here: you, me, the article
author, and Novice. The logging article author states that you should
never include file name, class name, or line number. To be accurate the
article is an edited compilation of what another fellow (Tomasz
Nurkiewicz) has written about logging: I haven't been able to find the
originals to see what Tomasz put forth as explanation for this stance.

Further, and presumably this also comes from Tomasz, it's stated that
"logging class name, method name and/or line number has a serious
performance impact." Now, to be fair to him this is also still
promulgated by Ceki Gulcu's log4j documentation.

I suspect that in recent years with newer JVMs that the performance
impact of some of the position conversion specifiers is considerably
less than it was when the warnings were first issued.

Having said that, *my* argument against regularly using source position
information is motivated by retaining clarity of logs. Try reading logs
in a text editor or on the console when the first part of every entry is
not only level and timestamp and thread ID but also FQ classname,
methodname, and line number. That positional info is irrelevant almost
all the time, and yet a _default_ decision to include it seriously
clutters logs. The info you really want starts halfway across the editor
window. The human eye is also not that great at differentiating one blob
of letters from another; ignoring the level and timestamp is easy (they
"look" different when scanning), but the positional info easily runs
together with the real message.

My position is, if you need it, include it. Don't default (let's say
through PatternLayout strings) to having it uniformly. Bear in mind (if
performance is still any kind of concern) that the log4j %c conversion
specifier gives you classname for free if you're creating loggers by
class. You don't need it for errors, not if you're dumping stacktraces.
You surely don't need it for info or warnings. Debug may or may not
benefit from positional information, but that should be case by case,
and if you do need a debug statement to include positional info, what's
so tough about putting it in that one, specific debug message?
I disagree. You should end the program gracefully and get someone to fix
the programming error right away.
[ SNIP ]

Not often in the real world can you do that, Lew. And you know it. One
source of work like this (fixing errors) for ops support and maintenance
programmers is operational problem reports (trouble tickets etc)
detected and entered by business. Another is ops support monitoring
(Splunk, say) of logs...in which case something they detect may also
enter the ticket/defect tracking systems.

Unless something is truly critical nobody is going to shut down a 24/7
public-facing or partner-supporting (b2b) system just because a
programming error popped up as a runtime exception. In fact, in the
absence of any problem reports from the field an issue like that might
well end up as a low/medium severity, low/medium priority defect - mixed
in with dozens or hundreds of others - that'll get handled a few weeks
or months later.

AHS
 
N

Novice

Lew said:
Novice wrote:
snip

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.


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?
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.
example:

logger.error(message, exception);
Exactly the kind of thing I had in mind.
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?
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?
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()?
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?
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.
Better, use 'IllegalArgumentException'.


'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.
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 ;-)
IllegalArgumentException exception = new
IllegalArgumentException(msg); logger.error("Unable to proceed",
exception);


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?
 
L

Lew

Arved said:
Lew wrote:
[ SNIP ]
We may be talking/thinking at cross-purposes here: you, me, the article
author, and Novice. The logging article author states that you should
never include file name, class name, or line number. To be accurate the
article is an edited compilation of what another fellow (Tomasz
Nurkiewicz) has written about logging: I haven't been able to find the
originals to see what Tomasz put forth as explanation for this stance.

Further, and presumably this also comes from Tomasz, it's stated that
"logging class name, method name and/or line number has a serious
performance impact." Now, to be fair to him this is also still
promulgated by Ceki Gulcu's log4j documentation.

I suspect that in recent years with newer JVMs that the performance
impact of some of the position conversion specifiers is considerably
less than it was when the warnings were first issued.

Having said that, *my* argument against regularly using source position
information is motivated by retaining clarity of logs. Try reading logs
in a text editor or on the console when the first part of every entry is
not only level and timestamp and thread ID but also FQ classname,
methodname, and line number. That positional info is irrelevant almost
all the time, and yet a _default_ decision to include it seriously
clutters logs. The info you really want starts halfway across the editor
window. The human eye is also not that great at differentiating one blob
of letters from another; ignoring the level and timestamp is easy (they
"look" different when scanning), but the positional info easily runs
together with the real message.

My position is, if you need it, include it. Don't default (let's say
through PatternLayout strings) to having it uniformly. Bear in mind (if
performance is still any kind of concern) that the log4j %c conversion
specifier gives you classname for free if you're creating loggers by
class. You don't need it for errors, not if you're dumping stacktraces.
You surely don't need it for info or warnings. Debug may or may not
benefit from positional information, but that should be case by case,
and if you do need a debug statement to include positional info, what's
so tough about putting it in that one, specific debug message?

I see. Good points.
I disagree. You should end the program gracefully and get someone to fix
the programming error right away.
[ SNIP ]

Not often in the real world can you do that, Lew. And you know it. One

At another point in that post, not cited, I talk about not ending the program
but, "Somewhere up the stack you should catch the exception and convert it to
valid program state."

You are right that that is often not ending the program. As I also said in
that post, in answer to when the program should abend, "Never. Program exit
should be under user control."

So I agree with you.
source of work like this (fixing errors) for ops support and maintenance
programmers is operational problem reports (trouble tickets etc)
detected and entered by business. Another is ops support monitoring
(Splunk, say) of logs...in which case something they detect may also
enter the ticket/defect tracking systems.

Unless something is truly critical nobody is going to shut down a 24/7
public-facing or partner-supporting (b2b) system just because a
programming error popped up as a runtime exception. In fact, in the
absence of any problem reports from the field an issue like that might
well end up as a low/medium severity, low/medium priority defect - mixed
in with dozens or hundreds of others - that'll get handled a few weeks
or months later.

As I also stated.

In the cases you state, "exiting gracefully" means "when the operator says
so". There is nothing in my statements to gainsay your points. /Au contraire/,
I spoke against exceptions crashing or stopping a program.

And you know it.
 
L

Lew

Novice said:
Lew said:
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.

Well, I didn't recommend that either. There are so many ways to avoid
redundant initialization. So - what could avoid the redundant initialization
without the pitfall you cite?
 
Ad

Advertisements

L

Lew

Novice said:
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?

I said *if* that's the strategy. What do you think? It may depend on the
specifics of the case.
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.

How would that exception ever occur? How could "someone" ever try to call a
private constructor?

Don't throw code into a program that is provably never called.
Tip 6 in the article makes the point that it is redundant and expensive

We've discussed this before, remember?

It's not expensive. When you're calling a logger statement for an error, the
cost of the error SWAMPS the time in the logger. Think about it.
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?

There are many who have answered this question, in response to your questions.

The answers covered a lot of approaches. Perhaps you recall them.
But what's the right way to end the program? System.exit()? That seems

Under operator control.
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?

Under operator control.
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()?

Wherever you detect the error.
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?

Yes.

It is common and frequently useful to create an application-specific checked
exception. Again, and I've said this many times, think about what will be
useful when troubleshooting a problem. Many times, a custom exception is
useful. It says that the underlying exception has been caught, logged and wrapped.

Runtime exceptions are dangerous because they slip past you. If you catch them
in a custom checked exception then code must handle it. All those runtime
exceptions are related, by dint of being exceptions within the same
application. BOOM! Qualified.
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.

That's his opinion.

Many others think that 'IllegalArgumentException' makes more sense if it's an
argument that has an illegal value.

What do you think?
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;

Kind of a bad name for the variable, there.

What does your log look like with 'msg' printed twice?
}

//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

Why, indeed?
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?

No, only the operator should stop the program.

The idea is to return to valid program state. RETURN TO VALID PROGRAM STATE.

*RETURN TO VALID PROGRAM STATE.*
Yes, I see that from your technique of creating one. I hadn't anticipated
that approach.

That's why Java has the 'new' operator.
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?

What do you think?

[snip]
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?

That depends. What constitutes valid program state? Should you just stop,
leaving the user wondering WTF happened? Should you perhaps come to a screen
that tells the user something went wrong, and give them some choices?

How do you feel when a program suddenly ends? What if you could've supplied
the missing "e" in the file name without having to start everything all over
again?

It all depends on the program, doesn't it?

Programs should really only end under operator control. But hey, do it your
way. What makes sense? What irritates the user? What's even possible?
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?

You tell me. The questions too vague. Describe the scenario precisely, with
the advantages and disadvantages of each approach. With more information about
your particular case, maybe I can advise.
 
A

Arivald

W dniu 2012-03-11 18:05, Novice pisze:
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.

Simplest solution, skip variable at all.

static public ResourceBundle getResources(
String baseName, Locale locale) {
try {
return ResourceBundle.getBundle(baseName, locale);
} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}
}


or, if you need to do something on variable:

try {
ResourceBundle locList = ResourceBundle.getBundle(baseName, locale);

... do something ...

return locList;

} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}


or if you want exception block only for ResourceBundle.getBundle()

ResourceBundle locList; // no null assignment, simply uninitialized
try {
locList = ResourceBundle.getBundle(baseName, locale);
} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}

do something with locList ...

return locList;
 
A

Arved Sandstrom

Arved Sandstrom wrote:
[ SNIP ]
As I also stated.

In the cases you state, "exiting gracefully" means "when the operator
says so". There is nothing in my statements to gainsay your points. /Au
contraire/, I spoke against exceptions crashing or stopping a program.

And you know it.
Fair enough: we agree. The point I was making was mostly for the OP, and
really does boil down to making it clear that often "exit gracefully"
means "when the operator says so", not having the application programmed
to automatically exit gracefully. I think it's valid in the OP's case to
make that distinction, seeing as how he's wondering why he hasn't seen
more System.exit calls.

AHS
 
A

Arved Sandstrom

Novice wrote:
[ SNIP ]
That's his opinion.

Many others think that 'IllegalArgumentException' makes more sense if
it's an argument that has an illegal value.

What do you think?
[ SNIP ]

StackOverflow has a good thread on this:
http://stackoverflow.com/questions/...-or-nullpointerexception-for-a-null-parameter.

Main takeaway is, in this case as in many others, as Lew suggests, you
do better to understand *why* people are making certain suggestions, and
then you can make your own choices. Most things in programming aren't
black and white.

Bear in mind, Bloch in "Effective Java" is trying to provide advice.
He's pretty good at it, but he's not infallible. Sometimes it shades a
bit too much over into "I say so...". I find it particularly rich that
he tells us that NPE is to be preferred when a parameter is null and
it's not allowed to be null, and the main reason is "convention
dictates": seeing as how he wrote a lot of JDK classes he had a big hand
in establishing the "convention", so this is quite circular.

Don't get me wrong: ignore Bloch or Goetz or Lea and other people like
that at your peril. But if you're experienced with technology X, and
have in fact been using it as long and probably as thoroughly as some
pundit, and you run across a statement by said pundit that you just
can't see the sense of, don't assume the other guy is right.

FWIW I use IllegalArgumentException in this case myself. I can't help
NPEs that are thrown by JDK classes even when I think they made a
mistake in choosing that exception, but for me an NPE means a
dereferencing failure, and if I've got the choice to create or rethrow,
I'll use IllegalArgumentException.

AHS
 
A

Arivald

W dniu 2012-03-11 19:07, Lew pisze:
Why, indeed?


No, only the operator should stop the program.

The idea is to return to valid program state. RETURN TO VALID PROGRAM
STATE.

*RETURN TO VALID PROGRAM STATE.*

Some errors are so bad, so only program termination is applicable. For
example when there is no valid state to recover.

Novice resource bundles it may be this case. If he have all GUI in XML
in bundles, or at least all language dependant data (GUI labels, menus,
etc), there is no valid state to recover. In this case program should
notify user (message box, with only OK button), and just die.

For console program, there is much more reasons to use exit(). Most
console programs just exit on error. Very few, usually complex ones,
try to recover.

Yet another case is server-side applet. In this case exiting on error is
standard strategy. Although in this case it exits through unhandled
exception, so server can handle termination reason.
 
Ad

Advertisements

N

Novice

Lew said:
Novice said:
Lew said:
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.

Well, I didn't recommend that either. There are so many ways to avoid
redundant initialization. So - what could avoid the redundant
initialization without the pitfall you cite?

I see that Arivald has answered that already in his contributions to the
thread ;-)

I'll use one of his approaches.
 
A

Arved Sandstrom

On 12-03-11 05:03 PM, Arivald wrote:
[ SNIP ]
For console program, there is much more reasons to use exit(). Most
console programs just exit on error. Very few, usually complex ones, try
to recover.
[ SNIP ]

I don't believe being a console program has anything to do with the
error handling strategy. If the program can't continue in any useful
form, except for making some last efforts at resource cleanup and some
logging, that's when you exit automatically on error. Console program or
otherwise. Otherwise you try to recover, or at least get to a state
where the operator can make some necessary salvage decisions before
being strongly advised to quit. But you don't automatically exit as a
matter of course.

"Console program" includes not just text-based UI but also CLI
operation. This covers a lot of ground, up to and including servers.
I've deployed and undeployed and bound apps on web and app servers, and
configured the servers, entirely through CLI operations and editing
configuration files, and no doubt so have you. That many of these apps
also have web and desktop GUIs doesn't hide the fact that the core app
is a console program. And you sure aren't going to exit on error with a
server, not unless it's a really serious error.

AHS
 
N

Novice

Lew said:
I said *if* that's the strategy. What do you think? It may depend on
the specifics of the case.
The bundle being sought is for use on the GUI. Displaying the GUI without
text is useless.

The nature of the error is a factor. If I'm passing a null for the
baseName, the program couldn't reasonably work out an alternate resource
bundle to use, nor could the user of the program. In that case, the
program should end with a SEVERE error written to the logs. If I'm
passing in a null locale, getResources() could determine the default
locale and use it, probably advising the user that it had done so and
logging that it had made this substitution. If the baseName were present
but misspelled, I don't see how the program could proceed.

So, in short, I could probably cope with a null locale but not a null or
misspelled baseName.

However, if the logic that I messed up in getLocalizedText() was supposed
to yield a language other than the one returned by Locale.getDefault()
(my proposed recovery strategy if locale is null in getResources(), the
GUI would display a different language than the user wanted. That might
be just as bad as a GUI without text if the user was, say, Japanese and
didn't know English, assuming English was the default locale.
How would that exception ever occur? How could "someone" ever try to
call a private constructor?

Don't throw code into a program that is provably never called.


We've discussed this before, remember?

It's not expensive. When you're calling a logger statement for an
error, the cost of the error SWAMPS the time in the logger. Think
about it.
I suppose I'm too easily swayed by other opinions. Arved, who seems very
knowledgeable, cited that article, which seemed to give it his seal of
approval. I don't know the author of the article but I tend to assume he
must be an expert to be asked to write about a Java topic. The author
cites people who he considers experts and the upshot is that the advice
seems to argue against putting positioning information in the log unless
its necessary (which it probably isn't if the stacktrace is in the log).
Now you make a persuasive argument contradicting the article. I'm
satisfied that you have serious expertise in this area so I have no
reason to dismiss your argument. But why didn't the experts who wrote the
article modify their remarks accordingly?

I am more than happy to include class name, method name and line number
in a log record. Those three things and the message text are the most
useful parts of the log record to me. They give me enough information
that I can find the right spot in the code and start to set up test
conditions to try to duplicate the error and find out what is causing it.
The stacktrace gives me that key information too but one of the questions
I haven't asked yet is whether I should always put a stacktrace in the
log for every problem situation. I'm inclined to think I should do so for
errors that get logged as SEVERE but not for FINE/FINER/FINEST messages.
WARNING and INFO messages are iffier and probably have to be decided on a
case-by-case basis. If I'm not writing a stacktrace, then I'll want the
log record to include the message, class name, method name and line
number at the very least and very likely things like the thread if I'm
doing multiple threads.
There are many who have answered this question, in response to your
questions.

The answers covered a lot of approaches. Perhaps you recall them.
I've had a lot of suggestions over the last few days and, in all honesty,
they are starting to blur in my mind.... Obviously, I need to reread some
of those threads....
Under operator control.


Under operator control.


Wherever you detect the error.
Good. That makes perfect sense to me.
Yes.

It is common and frequently useful to create an application-specific
checked exception. Again, and I've said this many times, think about
what will be useful when troubleshooting a problem. Many times, a
custom exception is useful. It says that the underlying exception has
been caught, logged and wrapped.
But how is a custom FooException better than a good ol'
IllegalArgumentException? I'm not quite getting that yet.

Please understand that I have no objection to creating my own exceptions.
I've actually done it in the past and didn't find it difficult.
Runtime exceptions are dangerous because they slip past you. If you
catch them in a custom checked exception then code must handle it. All
those runtime exceptions are related, by dint of being exceptions
within the same application. BOOM! Qualified.
It sounds a little bit like you're saying that RuntimeExceptions should
be made into chccked exceptions when possible. But I doubt you mean that.
That seems to contradict what Stelting at least is saying and even the
API. The spirit of the API seems to be that some things don't lend
themselves to easy recovery and you shouldn't even try, ergo the
existence of unchecked exceptions and errors. Naturally, some things DO
lend themselves to easy recovery by the user so it makes perfect sense
that if a given file isn't found, the user can be given a chance to
select a different file with a file chooser, which explains why
FileNotFoundException is a checked exception.
That's his opinion.

Many others think that 'IllegalArgumentException' makes more sense if
it's an argument that has an illegal value.

What do you think?
Like I said, I'm fine with either approach. They're both unchecked
exceptions and you'd handle them the same way. It feels like a matter of
personal style to me, meaning I'm free to do what I like on my own code
and I should follow the shop standard if I'm working for someone else.

Kind of a bad name for the variable, there.
You mean nullPointerException? I've gotten into the habit of giving my
instance names the same name as the class (except for the case of the
first letter) so I'm used to doing things that way. It seems clearer
since it avoids any possibly misunderstanding that this isn't just an
exception but is an instance of NullPointerException.
What does your log look like with 'msg' printed twice?
A bit redundant given that the same message appears at the top of the
stacktrace. But just having "SEVERE" without any other message seems like
a bad idea. What do you think I should have if in the log record (the
part that isn't the stacktrace) if not 'msg'?
Why, indeed?


No, only the operator should stop the program.

I think I need to be clear about who the operator is. In the case of a
game, say, we're talking about the person playing the game (assuming a
game that has only one player, like a solitaire game). Same kind of
reasoning for a utility like a program that burns CDs or whatnot. For a
servlet that is running in a container like Tomcat and may have hundreds
or thousands of instances running, we'd have both individual users of
individual copies of the program as well as some kind of admininstrator
managing the container. Have I got that about right?

If so, I'm not clear which operator you mean in the case of a servelet.
Assuming only one instance of an application was having trouble, I'd
expect the user to be the operator in question. In a case where all
instances of an application were having problems, I'd expect the
administrator of the container to shut down all instance of the problem
program pending a resolution.

If that's all more-or-less right so far, then what is the operator
experience going to be when the program has to deal with a null baseName
and the program is a game or utility? It would seem reasonable to display
a dialog with a "Sorry, we hit a snag and can't proceed" kind of message
that includes an okay button. It could go further and advise the user to
contact Tech Support and even display messages and diagnostics but I
expect most of that would be over the heads of users and should be
reserved for the logs. Then, when they press Okay, the program ends.

Is that about what you had in mind?
The idea is to return to valid program state. RETURN TO VALID PROGRAM
STATE.

*RETURN TO VALID PROGRAM STATE.*
I hear you; I'm just not sure what a valid program state would be in my
case. Honestly, I'm not entirely sure what "valid program state" implies
in ANY case ;-)


That's why Java has the 'new' operator.


What do you think?
I'm easy with either approach. It feels like a personal style thing to
me. The main thing is that the problem is detected and logged with enough
information that the operators and/or programmers can figure out what
went wrong. At the end of the day, the specific exception thrown doesn't
seem particularly important.
[snip]
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?

That depends. What constitutes valid program state? Should you just
stop, leaving the user wondering WTF happened? Should you perhaps come
to a screen that tells the user something went wrong, and give them
some choices?
Aside from not being clear about the meaning of "program state", I see
your point.
How do you feel when a program suddenly ends? What if you could've
supplied the missing "e" in the file name without having to start
everything all over again?
Absolutely. I think it would be dandy to enlist the user's assistance in
getting back out of trouble rather than crash the program on him.
It all depends on the program, doesn't it?
Yes.

Programs should really only end under operator control. But hey, do it
your way. What makes sense? What irritates the user? What's even
possible?
Fair enough. I'm all in favor of the user having a positive experience
rather than have them scratching their heads and muttering WTF!

In short, you've won me over. I think displaying a dialog and explaining
that we have to end is more than reasonable and is better than just
closing it on the user's behalf without any clear communication. My only
quibble is that the dialog is a bit of an empty gesture in the sense that
the user doesn't REALLY have much in the way of control when he gets a
dialog with only an Okay button on it. The program is already dead in the
water and his only option is to press Okay to officially kill the
program. That's not really "control" in a very strong sense. But I agree
that it is strongly preferable to do that than to just close it without
telling him what's going on in some way.
You tell me. The questions too vague. Describe the scenario precisely,
with the advantages and disadvantages of each approach. With more
information about your particular case, maybe I can advise.

I didn't really have a specific scenario in mind when I said that. It
just struck me that the answer would probably be different in a situation
like that than the case of a standalone utility or game. As you say, a
good programmer would consider that in devising his strategy for handling
the problem.
 
N

Novice

Novice wrote:
[ SNIP ]
That's his opinion.

Many others think that 'IllegalArgumentException' makes more sense if
it's an argument that has an illegal value.

What do you think?
[ SNIP ]

StackOverflow has a good thread on this:
http://stackoverflow.com/questions/3881/illegalargumentexception-or-nul
lpointerexception-for-a-null-parameter.

Main takeaway is, in this case as in many others, as Lew suggests, you
do better to understand *why* people are making certain suggestions,
and then you can make your own choices. Most things in programming
aren't black and white.

Bear in mind, Bloch in "Effective Java" is trying to provide advice.
He's pretty good at it, but he's not infallible. Sometimes it shades a
bit too much over into "I say so...". I find it particularly rich that
he tells us that NPE is to be preferred when a parameter is null and
it's not allowed to be null, and the main reason is "convention
dictates": seeing as how he wrote a lot of JDK classes he had a big
hand in establishing the "convention", so this is quite circular.
Excellent, excellent point ;-) As someone who still feels like a novice
about a lot of Java, I'm inclined to ascribe omniscience to the gurus and
I need to stop doing that. They put their pants on one leg at a time just
like I do and they are capable of being biased or even flat out wrong,
although I'm sure I'm wrong a LOT more than they are ;-)
Don't get me wrong: ignore Bloch or Goetz or Lea and other people like
that at your peril. But if you're experienced with technology X, and
have in fact been using it as long and probably as thoroughly as some
pundit, and you run across a statement by said pundit that you just
can't see the sense of, don't assume the other guy is right.
Excellent advice. And of course you've just restated Lew's advice in your
own words.
FWIW I use IllegalArgumentException in this case myself. I can't help
NPEs that are thrown by JDK classes even when I think they made a
mistake in choosing that exception, but for me an NPE means a
dereferencing failure, and if I've got the choice to create or
rethrow, I'll use IllegalArgumentException.
Like I said, I'm easy with either approach. Perhaps I'll use
IllegalArgumentException on even-numbered days and NullPointerException
on odd-numbered days just to show how even-handed I am ;-)
 
N

Novice

There are two issues, where to declare the variable and whether to
initialize it. The visibility issue means you have to declare it
outside the try/catch.

The issue is whether to initialize it in the declaration. If you do
that, you throw away some compile time checking. Does every path
leading to the return assign an appropriate value to locList?

There are a few situations in which a human programmer can tell that
all necessary assignments are being done, but the compiler cannot. In
those cases you may need to initialize in the declaration, but that is
not the way to start.

Even if I wanted to return null if you go through the catch block, I
would explicitly assign null in the catch block. Doing so shows future
readers, myself included, that I thought about the issue and meant it
to be null if the exception happened.
That's where I went wrong. As I recall, I got into the habit of
initializing every variable (with the odd exception) when I declare it
simply to avoid compiler warnings about the value potentially being null
later in the code. I didn't think it through very thoroughly and just
started slavishly initializing everything upon being declared. It
certainly worked for getting rid of compiler warnings but it wasn't
nearly as smart as figuring out when it needed to happen and when it was
ill-advised.

I'll need to start working on that....
How do you intend this method to be used? What does its Javdoc comment
say about missing resource bundles?
In short, if the baseName is null or misspelled, I can't think of a
plausible recovery strategy so I'm (now) inclined to show the user a
message that the program can't continue in a simple dialog with an Okay
button; when he presses that, the program gets closed.

I'm still struggling a bit with the missing locale. I _could_ determine
the default locale via Locale.getDefault() and it _might_ be different
than the one determined in the program, Foo, in which case I _could_ go
ahead and try to find the bundle for the default locale. But if it's a
different language than the one the user wanted, I'm not sure I'm doing
him a favor to show him a language he may not know. So I may treat the
null locale scenario as being just an unrecoverable as the missing or
incorrect baseName.
I generally prefer throwing exceptions to returning null in this sort
of situation. The reasoning is that the caller may want to take
different actions for different failures. Returning null, even after
logging the details, deprives the caller of any way of distinguishing
different cases. I realize this particular method currently only has
one error case, but that may change later.
Well, it's actually got three error cases: a null baseName, a null
locale, and a baseName that doesn't have a corresponding file, which is
caused by a spelling mistake in the base name. But I would certainly log
a distinct message in any of those cases.
Direct exit from a low level method is rarely a good idea. It limits
its usefulness. Generally, you should pass the buck up the stack, for
example by throwing an exception or returning null.

If you do that, your method can be used in contexts in which the
missing resource bundle is not fatal. Maybe the call arguments were
derived from GUI input. In that case, the user should be told about
the problem and invited to change the input. Maybe one transaction
depends on the resource bundle, and that transaction needs to be
aborted, but the program can keep on going.

Even in a batch program, you may be part way through processing some
input, and it is possible to do more validation, and find more input
errors, in a single run.

Or maybe there is a sensible default, and your caller knows what it
is. If you throw an exception or return a result that specifically
indicates missing resource bundle, the caller can use the default.
I see what you're saying. In my particular situation, the missing
resource bundle is a show-stopper but in other cases it might not be so
it makes more sense to let getLocalizedText deal with that than to handle
it in getResources(). And in some cases it might make sense to push it up
even higher, even right up to main().

Okay, on the theory that some other program MAY be able to carry on even
if the resources are missing - even though I can't think of a convincing
case right this minute - I'll let getResources() log the problem and
throw an exception, then let getLocalizedText() handle it. In my case,
it's going to show a simple "Program ending" dialog with an okay button
and pushing it is going to stop the program. That way, some future
program that can carry on without the resources will be free to respond
in some different way.
If you throw an exception, and this really is a situation in which the
correct action is to terminate the program, the exception will filter
up the stack to a level that knows, for example, that it is the main
method in a batch program. It can call System.exit with the
appropriate error code.
And that brings up a minor point that I've wondered about for a while. Is
there any standard approach to the integers that are used in System.exit
(). For instance, does a value of 16 typically indicate a severe error
while a 4 indicates a minor error? (I'm thinking of the old IBM mainframe
codes in my example.) It seems to me that the Java community may have
some conventions in this regard that I should follow....
 
Ad

Advertisements

N

Novice

W dniu 2012-03-11 18:05, Novice pisze:

Simplest solution, skip variable at all.

static public ResourceBundle getResources(
String baseName, Locale locale) {
try {
return ResourceBundle.getBundle(baseName, locale);
} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}
}


or, if you need to do something on variable:

try {
ResourceBundle locList = ResourceBundle.getBundle(baseName,
locale);

... do something ...

return locList;

} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}


or if you want exception block only for ResourceBundle.getBundle()

ResourceBundle locList; // no null assignment, simply uninitialized
try {
locList = ResourceBundle.getBundle(baseName, locale);
} catch (MissingResourceException e) {
logger.error('missing resource', e);
return null;
}

do something with locList ...

return locList;

Thanks, Arivald. I like your suggestions.
 
N

Novice

Generally, you should use different Exception subclasses for cases that
may need different handling, so that callers can set up appropriate
catch blocks.

Regardless of the conditions under which you choose to generate
IllegalArgumentException, many methods you will be calling use it to
indicate an illegal argument condition that has not yet been logged or
otherwise reported.

If you also use it to indicate exceptions that have already been logged,
you risk either repeated logging of the same issue, or failure to log.

On the other hand, if you create an Exception subclass that you only
throw after logging the underlying problem, there is no ambiguity.
Callers have to deal with the consequences of the failure, but should
only generate log messages if they have something to add. The underlying
problem has already been logged.
I'm sure this will seem like good advice when I understand it but it's
still vague to me.

It would really help if you could suggest a concrete example or two that
show when to use a custom exception and why it's better.
 
A

Arved Sandstrom

[ SNIP ]
I suppose I'm too easily swayed by other opinions. Arved, who seems very
knowledgeable, cited that article, which seemed to give it his seal of
approval. I don't know the author of the article but I tend to assume he
must be an expert to be asked to write about a Java topic. The author
cites people who he considers experts and the upshot is that the advice
seems to argue against putting positioning information in the log unless
its necessary (which it probably isn't if the stacktrace is in the log).
Now you make a persuasive argument contradicting the article. I'm
satisfied that you have serious expertise in this area so I have no
reason to dismiss your argument. But why didn't the experts who wrote the
article modify their remarks accordingly?
[ SNIP ]

The article did have my "seal of approval". That's to say, I read the
article, and agreed with most of it. Not 100 percent, but most of it.
And I didn't see anything in it which would lead a person down the
garden path. I recommend it because it's a useful read...but you
shouldn't accept it as the gospel, just as you shouldn't accept
"Effective Java" as the gospel either.

Logging is also a somewhat subjective matter. That's why different
opinions on different specifics. Ultimately TIMTOWTDI, to borrow a Perl
motto. What really counts is when the rubber meets the road, and the
logs have to be _used_, did the ops support people get what they needed
out of the logs, without too much anguish?

You've heard different viewpoints - mine and others' - on including
positional informational in logs. In the final analysis what matters is
the sufficiency of information in the logs. Don't worry overmuch about
logging performance until it's a problem; the whole point of having
layouts in a properties file is that you can tweak them if necessary. I
have a specific reason why I don't normally include certain positional
information, except for error stacktraces, but someone else might have a
good reason for why they do want this info.

One note: *never* assume that because somebody has an article in a blog
or an Internet magazine or a dead tree magazine or even a scholarly
journal that they are experts. Check the references. Check the style of
the prose: are the conclusions backed up by understandable argument, or
are they just "put out there"? Google for other articles on the same topic.

AHS
 
Ad

Advertisements

L

Lew

Novice said:
Patricia Shanahan :
I'm sure this will seem like good advice when I understand it but it's
still vague to me.

It would really help if you could suggest a concrete example or two that
show when to use a custom exception and why it's better.

This is treading into the matter of art. I can show the idiom, but no way can
I generate an entire project for you to show the impact of the logging
strategy across the project, which is the point of the idiom.

Your entire application has layers. The deeper low-level layers are most
likely to encounter problems first. Runtime exceptions are indicative of
programming errors, but they're slippery, too. You don't have to catch them,
which means an unguarded runtime exception can bring your system crashing
down. That's not something you want to design it to do.

A low-level layer by definition brings some model of the world into
conformance with your application's model. It might read a file and extract
data to objects, or persist objects to a database, or transform your
application's model into and out of XML documents for communication to other
modules. Whatever the low layer does, the higher-level client code deals with
abstractions relevant to the program. It's no longer a database record, for
example, it's a 'Customer' with related 'Order' elements in a collection.

If something goes wrong at a deep layer, the higher-level client must first,
become aware of it, and second, have some way to handle it. At that level,
"runtime exception" or 'IOException' are too low-level - they aren't in the
semantics of the application model. So the lower level, in addition to
transforming extrinsic models of good stuff into the application domain, also
must transform bad stuff like exceptions.

One good way to do this is to have an application-specific checked exception
like 'FooException' for the "Foo" application. As Patricia says, this marks
for higher-level code that lower-level stuff has done its job. At the higher
level, we care only that the details are packaged in the exception's 'cause',
not what the details are. A 'FooException', being checked, must be caught, so
it cannot accidentally crash the application. Since all higher-level code sees
'FooException' as a marker for bad stuff, and a black box, they all do the
same thing with it, more or less. They return the program to valid state

"state" - definable by a quick Google search, for Pete's sake! Come on, man!
Do a /little/ of the lifting yourself. And no, this isn't "bludgeoning". You
get your ego in the way and you won't learn. There's a reason we expect you to
act like a professional.

such as a decision screen or restart position, perhaps extracting standardized
fields from the 'FooException' for the error message.

Much of programming only makes sense if you think architecturally - that is,
holistically. Each layer has its own job, and its own vocabulary for that job.
"Foo" has no business knowing about "I/O" in its Foo-specific logic, and no
business knowing about "Foo" behavior in its I/O logic.

Separation of concerns.
Law of Demeter.
Object oriented.
Modularity.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top