In my opinion, there are some cases where throwing an NPE is
appropriate. As an example, Validate.notNull() from Apache Commons Lang
throws a NPE if the argument is null
(http://commons.apache.org/proper/commons-lang/javadocs/api-release/org/apache/commons/lang3/Validate.html#notNull%28T%29).
Throwing the exception early in the program flow makes the problem
easier to trace than if the null value is used later.
However, I rethought the NPE usage and now think an
IllegalStateException would be more appropriate. Finding the defining
parent is essential to the correct working of the rule. If it can't be
found (a bug in the rule code), the rule is in an invalid state and
should fail hard and fast. Creating an own exception type is overkill in
my opinion, because the IllegalStateException (with a descriptive
message) is meaningful enough by itself, and in any case should never be
thrown when all bugs are fixed ;-)
I'd go with whatever you suggest, though. Perhaps Mirko should make the
decision, because he wrote the rule and is a committer.
Cheers
Michael
On 15.05.2013 09:49, Baptiste MATHUS wrote:
+1. I don't think there's any case (in mojos, or even anywhere) where
manually throwing a NPE is valid.
Cheers
2013/5/14 Robert Scholte <[email protected]
<mailto:[email protected]>>
The fact that you throw the NPE yourself means that it is an
expected Exception.
If you read EnforcerRuleException as "an/any Exception in this
EnforcerRule", it is valid for me to throw this kind of Exception,
although I can imagine the doubt.
If you don't want to use the ERE, then create a new
RuntimeException, matching the real issue, like
MissingParentException. I agree with several code quality checkers
that you should avoid to throw NPEs yourself.
Robert
On Tue, 14 May 2013 09:26:48 +0200, Michael Koch
<[email protected] <mailto:[email protected]>> wrote:
Throwing a NPE yourself? There must be a better solution....
+
+ // fail fast if the defining parent could not
be found due to a bug in the rule
+ if ( parent == null )
+ {
+ throw new NullPointerException( "failed to
find parent POM which defines the current rule" );
+ }
This code is originally from me, so I'll explain. If the rule
fails to find the parent this is due to an error in the rule
code. The explicit check makes it easier to diagnose. Without
it, a NPE will be thrown later when the parent value is needed.
This is not always the case: without the explicit check the
plugin execution will succeed when used in the parent POM, but
fail with a NPE in the child POM which inherits the configuration.
It is debatable whether NullPointerException is a good choice,
but I think none of the other standard RuntimeExceptions are a
better fit. I also didn't want to use EnforcerRuleException,
because the NPE is an internal error, not an expected result of
the rule configuration / execution.
Cheers
Michael Koch
------------------------------__------------------------------__---------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/__manage_email
<http://xircles.codehaus.org/manage_email>
------------------------------__------------------------------__---------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/__manage_email
<http://xircles.codehaus.org/manage_email>
--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
