commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Lambrou <m...@chrislambrou.com>
Subject Re: [collections] Java5.0 - Runtime exceptions or assertion errors. What's your preference?
Date Sat, 06 Nov 2004 03:11:26 GMT
Hi,

As much as I'd like to replace all of the existing parameter checks with 
assertions, I think that the current method of having explicit parameter 
checking on all public methods will have to stand.  Many thanks to you 
all for your useful input.  I thought that for the sake of completeness, 
I'd tell you my own take on the issue. If you're interested, read on, 
but otherwise you should stop reading here.

Chris




My preference would have been to use assertion checks in the code, 
rather than the existing set of checks.  I've rejected this for the 
following reasons.

1. The standard Java collection interfaces mandate the throwing of NPEs. 
Using assertions would violate this.
2. Whilst Sun claims that disabled assertions have a negligible affect 
on runtime performance, I don't especially believe that null pointer 
checks have a great impact on performance either. Furthermore, for the 
kind of parameter checks that take place in collections, I'm not 
convinced that the ability to disable assertions would yield a  
noticeable increase in performance.
3. No matter how well the code is unit tested, there's no guarantee that 
library won't be bug free. In keeping with the principle of trying to 
detect errors as early as possible, it's best to keep the checks in at 
all times, rather than allow them to be disabled (by default) during 
normal use.
4. I like to use assertions to catch and eliminate problems during 
development and testing, and disable them the code is used in a shipped 
product. However, I can't force users of collections15 to follow the 
same development practice. Since these checks are to detect errors in 
the user's code, rather than in collections15 itself, it's best to keep 
the checks in at all times, rather than to use assertions.


Frank W. Zammetti wrote:
It is an accepted usage pattern that assertions are not to be used to 
validate method parameters if they are part of the public interface. 
They are properly used to check parameters passed to private members 
though, but nothing a calling client might send in.

I generally agree.  This is the approach I've settled on.


David Graham wrote:
The main reason not to use assertions is that they can be disabled at 
runtime, rendering them fairly useless IMO.  Personally, I would remove 
the null parameter checking entirely and let the code throw a NPE.  The 
caller will see their offending method in the stack trace.

Maybe they will, but only if the error shows up. Following the principle 
of detecting errors as early as possible, I think it's much better to 
have an explicit check on the parameters, so it's easier to pinpoint 
when and where the error occurred. For example, if the error checking of 
arguments is in the constructor of a class, then the error will show 
itself when and where the constructor is invoked, rather than the first 
time the instance is used in such a way as to cause it to fail.


Martin Cooper wrote:
Assertions are actually disabled by default, and need to be explicitly 
enabled at JVM startup time. To make things worse, specifying a flag to 
the JVM on startup can be complicated when an app server is involved.

Point taken.  Another good reason for not using assertions for this 
particular purpose.

In my day job, we looked into using Java assertions when we could 
finally depend on JDK 1.4 or later, and chose to eschew them in favour 
of our own homegrown assertion class for the reasons above.

Collections15 is too low level for it to warrant having such a 
dependency on another library.


Matthew Hawthorne wrote:
I think this was a fundamental goal of assertions.  I imagine that the 
idea is to enable assertions during heavy development time, and disable 
them once the code reaches a certain degree of stability.

This is precisely the reason I was considering the use of assertions.

The enable/disable concept is appealing to some for performance reasons. 
 I think Sun claims that an assertion will have no performance cost if 
they are disabled.  I'm not sure how much of a difference it would make.

Me neither.  I just can't see simple null checks causing a noticeable 
degradation in performance. Even if they did, I think the benefits of 
their presence generally outweigh such a cost.

I share your philosophy on this.  In my opinion, the redundant parameter 
checking everywhere (especially for null) does more harm than good.

I have to disagree. The only harm I can see is a slight performance 
decrease (which I suspect is negligible for simple null checks), and 
class files which are a bit larger than they otherwise would be without 
the checks.  The additional validation helps to detect bugs earlier 
rather than later, which I think is a benefit that far outweights the 
cost.  The thinking behind this is in line with my earlier response to 
David Graham's comment.

I like assertions, except in cases of input checking for a client 
external to the application (like a web service client, for example).

Me too.  This is the approach I've settled on.

I don't see the enable/disable as much of an issue.  The 
RuntimeException cases are supposed to be caught during development anyway.

That's how I like to develop, but I can't force programmers that use 
collections15 to follow the same principle.


Stephen Colebourne wrote:
Personally, I strive to avoid any NPE, preferring IAEs instead, but not 
everyone in commons agrees with that philosophy. collections15 should 
make one decision on that and stick by it.

I agree.  To my mind, the very name IllegalArgumentException tells me 
that this is the Exception type for a method to throw if it detects 
incorrect input.

Bear in mind that the JDK interfaces mandate NPE for null inputs.

I think that Sun have got this wrong. They should have mandated IAE, not 
NPE. In parctice, however, I don't think it makes much difference to 
client code, since both are unchecked.  I intend to throw IAEs 
throughout collections15, except where the Java Collections interfaces 
mandate NPEs instead.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message