Choosing not to throw exceptions like IllegalArguementException

[Robert]

Hi,
id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

On the other hand, it requires a better knowledge of the internals when
a bug a seen, since the program doesn't blow up compeltely.

 

[Tom Anderson]

id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

It doesn't. It causes subtle and incredibly hard-to-find bugs. Whether the
method is returning null or throwing an exception, it's not doing the job
it was called to do, and that means the program isn't working properly.

If a method is being passed nulls when it shouldn't be, or any other
inappropriate arguments, the correct thing to do is for it to complain
loudly. Then for you, when it happens, to find the code that is passing in
the inappropriate arguments and fix it.

tom

 

[Arved Sandstrom]

Hi,
id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

On the other hand, it requires a better knowledge of the internals when
a bug a seen, since the program doesn't blow up compeltely.

Quite frankly I'd like to see an example of this practise...just a short
code snippet that illustrates what you have in mind. You can certainly have
valid null argument values, and deal with them in a given way for a
particular situation - the equals() method does this. But as a general
rule?

It's the way you phrased it - it sounds like you are returning a default
value instead of throwing IllegalArgumentException when a null argument
actually is an error. And this is not good practise. You have to document
what the default value is, and since it signals an error you have to look
for it and deal with it anyway. You also have to be careful that no valid
argument produces a return value which is identical to the default. This is
not robustness.

Granted, your program may not blow up, but maybe your boss' stack will...

AHS

 

[Robert]

Arved Sandstrom a écrit :

Quite frankly I'd like to see an example of this practise...just a short
code snippet that illustrates what you have in mind. You can certainly have

1/ It won't happen :

private Point positionDefault(Clazz o) {
Point posRel = null;

if (o instanceof ClazzA)
posRel = new Point(5, -45);
else if (o instanceof ClazzB)
posRel = new Point(5, 45);

posRel.width += 100; // null warning by eclipse
...


2/ ClazzA code is now the default:

private Point positionDefault(Clazz o) {
Point posRel = new Point(5, -45);

if (o instanceof ClazzB)
posRel = new Point(5, 45);

posRel.width += 100;
...


3/ Bad, bad, bad

private Point positionDefault(Clazz o) {
Point posRel;

if (o instanceof ClazzA)
posRel = new Point(5, -45);
else if (o instanceof ClazzB)
posRel = new Point(5, 45);
else
throw new IllegalArgumentException();

posRel.width += 100;
...

valid null argument values, and deal with them in a given way for a
particular situation - the equals() method does this. But as a general
rule?

It's the way you phrased it - it sounds like you are returning a default
value instead of throwing IllegalArgumentException when a null argument
actually is an error. And this is not good practise. You have to document
what the default value is, and since it signals an error you have to look
for it and deal with it anyway. You also have to be careful that no valid
argument produces a return value which is identical to the default. This is
not robustness.

Our dev team uses this for GUI code mostly.

Granted, your program may not blow up, but maybe your boss' stack will...

I didn't get it.

 

[Patricia Shanahan]

Hi,
id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

That depends on whether continued operation is more or less important
than correct operation. For most programs I've worked on, silent wrong
answers were far worse than a crash.

At the worst, it could result in incorrect data being written to an
important database, an outcome with far worse long term consequences
than having a program crash and restart.

On the other hand, it requires a better knowledge of the internals when
a bug a seen, since the program doesn't blow up compeltely.

The worse problem is that the error may not even get logged or noticed,
preventing debug from happening at all. At the best, it indeed seems
like a recipe for converting easily debugged problems into subtle wrong
answer problems.

Patricia

 

[Robert]

Patricia Shanahan a écrit :

That depends on whether continued operation is more or less important
than correct operation. For most programs I've worked on, silent wrong

So true!

answers were far worse than a crash.

At the worst, it could result in incorrect data being written to an
important database, an outcome with far worse long term consequences
than having a program crash and restart.
Yes



The worse problem is that the error may not even get logged or noticed,
preventing debug from happening at all. At the best, it indeed seems
like a recipe for converting easily debugged problems into subtle wrong
answer problems.

It's harder to debug, but it worth it for GUI code i think

 

[Tom Anderson]

Arved Sandstrom a écrit :

1/ It won't happen :

private Point positionDefault(Clazz o) {
Point posRel = null;
if (o instanceof ClazzA)
posRel = new Point(5, -45);
else if (o instanceof ClazzB)
posRel = new Point(5, 45);

posRel.width += 100; // null warning by eclipse
...

2/ ClazzA code is now the default:

private Point positionDefault(Clazz o) {
Point posRel = new Point(5, -45);
if (o instanceof ClazzB)
posRel = new Point(5, 45);

posRel.width += 100;
...

3/ Bad, bad, bad

private Point positionDefault(Clazz o) {
Point posRel;
if (o instanceof ClazzA)
posRel = new Point(5, -45);
else if (o instanceof ClazzB)
posRel = new Point(5, 45);
else
throw new IllegalArgumentException();

posRel.width += 100;
...

Why is this bad?

The question here is what this method *should* do if given an instance of
a class other than A or B. If the answer is "that can't happen, it's
always A or B", then throwing an exception is the only right thing to do,
because it signals that something impossible has happened. If it's the
case that other classes are, or might one day be, possible here, then
having a sensible default and special-casing the classes which need it is
not a bad idea.

Of course, in this particular situation, you should really be using
polymorphism, not an if-else cascade, but this is an example for the sake
of argument, so never mind.

I didn't get it.

If your programs don't crash, but operate incorrectly instead, your boss
may become very angry.

tom

 

[Patricia Shanahan]

Patricia Shanahan a écrit : ....


It's harder to debug, but it worth it for GUI code i think

But does it improve even GUI code?

First consider what happens with exceptions. It should be possible to
organize the deployment so that any uncaught Throwable or hang causes a
restart of the application. From the point of view of the end user, the
screen stops working for a few seconds and then goes back to the initial
page. The event is visible to the people managing the application, and
gets logged with information such as the exception stack trace.

If the application is competently supported, something that causes that
sort of log entry should get fixed quickly. Even if the log does not
give enough data for immediate resolution, it gives enough information
to enable instrumentation of the method that threw the exception and its
caller.

On the other hand, consider the silent wrong answer case. End users make
vague reports about the application doing something other than what it
was told to do. Often, the end users will not realize there is anything
wrong until some time afterwards. The people managing the application
may not even realise there is a bug, but end start feeling the site is
flaky and avoiding it.

Patricia

 

[Robert]

Patricia Shanahan a écrit :

But does it improve even GUI code?

First consider what happens with exceptions. It should be possible to
organize the deployment so that any uncaught Throwable or hang causes a
restart of the application. From the point of view of the end user, the
screen stops working for a few seconds and then goes back to the initial
page. The event is visible to the people managing the application, and
gets logged with information such as the exception stack trace.

I don't think it's a good idea. We are developping a desktop
application, and an "application restart" is impossible. Imagine the
TreeCellRenderer wich throw an exception, and prevent the JTree from any
repaint success, only because some bug happened before, which broke a
specific assumption, and now there is the "impossible case" happening.

This is "bug handling" since it assumes the less it can.

If the application is competently supported, something that causes that
sort of log entry should get fixed quickly. Even if the log does not
give enough data for immediate resolution, it gives enough information
to enable instrumentation of the method that threw the exception and its
caller.

On the other hand, consider the silent wrong answer case. End users make
vague reports about the application doing something other than what it
was told to do. Often, the end users will not realize there is anything
wrong until some time afterwards. The people managing the application
may not even realise there is a bug, but end start feeling the site is
flaky and avoiding it.

I think that's the point, users won't realize there was a bug, because
they will try a second time, from a previous state, with hopefully more
success.

 

[Daniel Pitts]

Patricia Shanahan a écrit :

I don't think it's a good idea. We are developping a desktop
application, and an "application restart" is impossible. Imagine the
TreeCellRenderer wich throw an exception, and prevent the JTree from any
repaint success, only because some bug happened before, which broke a
specific assumption, and now there is the "impossible case" happening.

This is "bug handling" since it assumes the less it can.


I think that's the point, users won't realize there was a bug, because
they will try a second time, from a previous state, with hopefully more
success.

Users appreciate having the program do what they expect, or tell them
why it couldn't. You're approach to error handling allows a third
option; don't do what the user expects, and don't tell them why. Whoops.

Truth be told, these types of bugs should be caught in your unit-tests,
and the exceptions left in place. GUI code is often harder to
unit-test, but the point is that if you can't come up with a sensible
default value, don't come up with a nonsense result. Users will be more
disappointed with nonsense than with error messages.

 

[Roedy Green]

Sometimes it's better to return a value, sometimes it's better to throw an
exception.

If you return a value your caller must check for and deal with the
problem. If you throw an exception, your caller can fob dealing with
the problem to its caller.

If you return a magic value, you must have the discipline to always
check. In working with C++ teams, I found this very difficult to
enforce.

 

[Joshua Cranmer]

Hi,
id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

On the other hand, it requires a better knowledge of the internals when
a bug a seen, since the program doesn't blow up compeltely.

Ultimately, I think the question of throwing an exception or returning a
value comes down to one key point: is the situation "expected"? For
example, it would not be an exceptional circumstance to use List.indexOf
on an object not included in the list, so an exception here does not
make sense. On the other hand, it would be exceptional to open a
connection to a nonexistent server, so an exception there does make sense.

The question I ask to you is "Does an argument of null imply retrieving
the default value (or other special code), or is it something that
should not be occurring?" In my experience, the former is of quite
limited utility, mostly limited to some sort of configuration semantics.

 

[Stefan Ram]

make sense. On the other hand, it would be exceptional to open a
connection to a nonexistent server, so an exception there does make sense.

This criterion seems to be somewhat arbitrary, because it
depends on subjective assessment.

When a well-educated programmer attempts to open a connection
to a server he will expect a piece of information from the
service used to open the connection that indicates whether the
attempt has succeeded.

He will not take it for granted that such an attempt will
succeed at run time.

~~

There are points when exceptions are not expected indeed.
For example, when a service is activating an arbitrary
client operation:

startClientOperation( final java.lang.Runnable clientOperation )
{ clientOperation.run(); }

Here the author of »startClientOperation« does not know what
»run()« will do, so he can not expect anything about it.
The operation might throw runtime exception.

Another example would be a situation, when an operation
that does not throw any exceptions needs to be modified to
throw an exception. It would break clients to use a checked
exception for this, but a runtime exception can be used.

So it seems that whenever the author of a client can not know
whether an exception might be thrown, a runtime exception seems
to be appropriate, while the author of a client must be
aware of checked exceptions.

 

[Arved Sandstrom]

Ultimately, I think the question of throwing an exception or returning a
value comes down to one key point: is the situation "expected"? For
example, it would not be an exceptional circumstance to use List.indexOf
on an object not included in the list, so an exception here does not make
sense. On the other hand, it would be exceptional to open a connection to
a nonexistent server, so an exception there does make sense.

The question I ask to you is "Does an argument of null imply retrieving
the default value (or other special code), or is it something that should
not be occurring?" In my experience, the former is of quite limited
utility, mostly limited to some sort of configuration semantics.

"Something that should not be occurring" is when runtime exceptions come in.
For the other situations you describe we often do use checked exceptions,
for example FileNotFoundException.

Collection classes are maybe not the greatest example, since the contracts
allow for the possibility of ineligible elements (including null) for some
implementations, and hence one class may return a default and another throw
an exception, for the same method in each.

I'd put it rather like this - use unchecked exceptions for "this is flat-out
wrong and/or we can't recover". Use checked exceptions for "this is expected
and we can recover from it". Possibly use unchecked exceptions if we have
"this is expected but we don't usually want to recover from it". Default
return values make sense in the case when there is actually a useful
default, not so much as a replacement for an exception.

Current Java APIs exhibit varied degrees of anarchy in this regard.

AHS

 

[Mark Space]

Users appreciate having the program do what they expect, or tell them
why it couldn't. You're approach to error handling allows a third
option; don't do what the user expects, and don't tell them why. Whoops.

Usually when I press the save button, I prefer the program to just put
the file in some random location, rather than where I asked it to.

Or not.

Truth be told, these types of bugs should be caught in your unit-tests,
and the exceptions left in place. GUI code is often harder to
unit-test, but the point is that if you can't come up with a sensible
default value, don't come up with a nonsense result. Users will be more
disappointed with nonsense than with error messages.

Yes, or later than unit tests (integration tests, system tests). "Don't
throw errors" is applicable only for the highest level code. Even then,
the program should probably fail gracefully and offer a bug report
screen, because if anything does propagate that far, it's a serious problem.

At lower levels in code, not throwing error will prevent the programmers
and testers from discovering errors at all, and fixing them.

A more correct scheme would be at the lowest levels to throw all
applicable errors so they can be discovered in unit tests. At the more
coarse interfaces, catch errors and offer a message to the user ("You
can save a file there because you don't have write permission."), or log
it. At the highest level, offer a panic message (usually with bug
report form for the user) because these are errors that should not have
escaped the coarse interfaces.

Pretty trite, but a better starting point than what the OP is doing, I
think.

 

[Paul Tomblin]

Hi,
id' like to share with you a design view i've learned since i work. When
i have to deal with nulls, i don't throw an IllegalArguementException
any more, i just return a default value (or void). It gives better
robustness to the program i think.

Yeah, because back when we programmed in C and the only indication of an
error was a value in errno, programs *never* failed.

 

[Arne Vajhøj]

This criterion seems to be somewhat arbitrary, because it
depends on subjective assessment.

Absolutely.

It is one of those cases where the developer has to use his/her
judgment. There is 3 zones: everybody will choose A, some will
choose A and some will choose B, everybody will choose B. I will
postulate that the second zone will become smaller the more
experienced the programmers are.

Arne

 

[Stefan Ram]

When a well-educated programmer attempts to open a connection
to a server he will expect a piece of information from the
service used to open the connection that indicates whether the
attempt has succeeded.

In another context, I read a sentence reminding me of this.

»Nothing's an emergency in an emergency room because you expect it.«

http://www.boston.com/news/science/...go_to_guy_in_mental_health_studies/?page=full

 

[Robert]

Lew a écrit :

Actually, if the only reason you're throwing an exception is that you're
using explicit run-time type tests, instead of relying on
object-oriented programming and compile-time checking, then you have a
deeper problem than whether you should silently eat and ignore exceptions.

That example actually shows that one should not be testing for type
explicitly, or else one is stuck with handling an exception that
shouldn't even be possible. Using polymorphism as Tom suggests obviates
the entire problem.

As an example of whether one should throw exceptions it totally fails,
because it relies on a scenario that one should prevent in the first place.

Do you think having an overrided method with return a constant Point
object worth it ?

Don't you think it makes the original method harder to read ?

 

[Hendrik Maryns]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert schreef:
| Lew a écrit :
|> Tom Anderson wrote:
|>> The question here is what this method *should* do if given an
|>> instance of a class other than A or B. If the answer is "that can't
|>> happen, it's always A or B", then throwing an exception is the only
|>> right thing to do, because it signals that something impossible has
|>> happened. If it's the case that other classes are, or might one day
|>> be, possible here, then having a sensible default and special-casing
|>> the classes which need it is not a bad idea.
|>>
|>> Of course, in this particular situation, you should really be using
|>> polymorphism, not an if-else cascade, but this is an example for the
|>> sake of argument, so never mind.
|>
|> Actually, if the only reason you're throwing an exception is that
|> you're using explicit run-time type tests, instead of relying on
|> object-oriented programming and compile-time checking, then you have a
|> deeper problem than whether you should silently eat and ignore
|> exceptions.
|>
|> That example actually shows that one should not be testing for type
|> explicitly, or else one is stuck with handling an exception that
|> shouldn't even be possible. Using polymorphism as Tom suggests
|> obviates the entire problem.
|>
|> As an example of whether one should throw exceptions it totally fails,
|> because it relies on a scenario that one should prevent in the first
|> place.
|>
|
| Do you think having an overrided method with return a constant Point
| object worth it ?
|
| Don't you think it makes the original method harder to read ?

Why don’t you give an example. Such questions are hard to answer and
differ from case to case. In general I’d say: no, since I assume you
(or the casual reader of the code) know how the overriding mechanism
works and as such can easily understand what is going on.

H.
- --
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
http://aouw.org
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFITQPJe+7xMGD3itQRAomyAJ9NnNkrqrJX992xlIQZYA8NUqjNSQCeLjYF
WddvKSa+CLwDq/ysFtAqw7I=
=PxU1
-----END PGP SIGNATURE-----