[jira] [Created] (MATH-856) Deprecate "NullArgumentException"

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Created] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
Gilles created MATH-856:
---------------------------

             Summary: Deprecate "NullArgumentException"
                 Key: MATH-856
                 URL: https://issues.apache.org/jira/browse/MATH-856
             Project: Commons Math
          Issue Type: Task
    Affects Versions: 3.0
            Reporter: Gilles
            Priority: Trivial
             Fix For: 3.1, 4.0


[Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.

It will be deprecated in 3.1 and removed in 4.0.

Checks for "null" in CM code will either be maintained or be removed.

Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)

    [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13450877#comment-13450877 ]

Phil Steitz commented on MATH-856:
----------------------------------

+1 for deprecating NAE, but maintaining null-checking where it exists now and for APIs that can benefit from full parameter checking.
+0 for allowing APIs that cannot reasonably check for and document null handling behavior to just allow NPEs to propagate untrapped, undocumented and unadvertised
-1 for throwing argumentless NPEs via APIs that check for null arguments and document null behavior

I think we should continue to throw IAE in the last case above, with informative messages where appropriate.  Eliminating NAE in this case will result in APIs that currently check for nulls and throw a NAE with localized message to instead throw MathIAE with the same message.
               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
In reply to this post by Dmitri Blinov (Jira)

    [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13451052#comment-13451052 ]

Gilles commented on MATH-856:
-----------------------------

I don't agree with the above set of alternatives.

* Either we throw a standard NPE exception, which is not localizable since the localization happens at the call to "getLocalizedMessage" with the support of the CM-specific "ExceptionContext" (and since French messages are mandatory for some of Luc's applications, this option mandates the no-arg constructor),
* or we want to keep the possibility to check and report illegal usage of "null", with a localizable message; for this, "NullArgumentException" is appropriate (since it is a subclass of "MathIAE", it fulfills the requirement of your last paragraph).

Thus, we should not deprecate "NullArgumentException" after all (?), but we'd have to carefully set rules for when the check for "null" should or should not be done, lest contributors (new and old) will not get it right.

I'm still +1 for not checking for "null", whatsoever: It is a trivial bug. And having an explicit check (instead of just letting the JVM do it) will not make it easier for the user to answer the question: "Where did I forget to initialize that variable?"; he'll have to go to the stack trace anyways.

I don't think that anything is gained from checking for "null" and not throwing NPE (IAE is not NPE, MathIAE is not NPE, NullArgumentException is not NPE and soon won't be IAE anymore). From this will come only confusion.

               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
In reply to this post by Dmitri Blinov (Jira)

    [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13451111#comment-13451111 ]

Phil Steitz commented on MATH-856:
----------------------------------

Then lets leave things as is.  I do not want to reduce robustness in places where we document null handling and throw IAE with informative messages where not allowed nulls are provided. We can't agree to document null handling throughout, so lets just agree to leave the more robust bits alone.  Propagating runtime exceptions with no warning is a bad practice, IMO, but I understand that other developers do not want to add null checking everywhere and I am not going to insist on it.   A null array, an empty array, a bad set of indices are all the same in most places in the stats package - all violate the fully specified API contract and lead to informative IAEs.  The NAE is needless, but harmless.  If we can't agree to drop it but replace with MathIAE, then lets leave as is.
               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
In reply to this post by Dmitri Blinov (Jira)

    [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13451292#comment-13451292 ]

Gilles commented on MATH-856:
-----------------------------

bq. I do not want to reduce robustness in places where we document null handling and throw IAE with informative messages where not allowed nulls are provided.

The issue which we started to discuss on the ML is whether checking for "null" is useful or not.
The majority seems to agree that it is not.

The code will _not_ be less robust without CM checking for null, since such bugs will raise a NPE generated by the JVM.

bq. Propagating runtime exceptions with no warning is a bad practice [...]

I don't understand what you mean by "no warning". Everybody knows that Java will raise a NPE when a null reference is used.

The point is indeed that _we_ don't propagate (unless there is a bug in CM where _we_ created and subsequently used a null reference).

bq. [...] developers do not want to add null checking everywhere [...]

That's the crux of the issue. Doing so is duplicating the JVM check with no added value: the information is not more detailed; it is just the same ("a null reference was used").
The sole difference is that the exception might be raised one or two levels up the stack.
This gain (of dubious usefulness) must be balanced with the (even slight) performance loss incurred by checking for null everywhere.

bq. A null array, an empty array, a bad set of indices are all the same in most places in the stats package - all violate the fully specified API contract and lead to informative IAEs.

I never proposed to drop checks for
* empty array
* bad set of indices

In those cases, the reason for the error cannot be inferred from just _raising_ an exception; there is undoubtedly an added value to a more specific exception, with a more detailed message (if needed) and, possibly, additional information stored in the "ExceptionContext".

Just raising NPE _is_ enough to convey the reason with full meaning disclosure: a null reference was used.

bq. The NAE is needless, [...]

The statement is in contradiction with the usage you want to maintain.
NAE exists to encapsulate the localizable error message ("LocalizedFormats.NULL_NOT_ALLOWED").

bq. If we can't agree to drop it but replace with MathIAE, [...]

NAE inherits form MathIAE, thus it *is* a MathIAE, it can be used as a MathIAE, and it is more useful than MathIAE (for developers and users alike). There isn't a single advantage to using MathIAE over NAE.
If some null checks remain and must be reported, why would you want to delete this class, which does the same job as MathIAE but more concisely?

               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
In reply to this post by Dmitri Blinov (Jira)

    [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13451346#comment-13451346 ]

Phil Steitz commented on MATH-856:
----------------------------------

Probably better to take this discussion back to the mailing list, as we do not have consensus on what we are going to do.  

I am OK with the conclusion that it is not practical for every class in [math] to advertise all exceptions raised or propagated by the code.  I just don't want to reduce robustness in documentation and parameter checking where it exists today.  Just "letting the JVM handle null references" means unexpected NPEs that can cause problems for applications. Same applies to array index exceptions and all other kinds of RTE that can be triggered by the code if parameters are not checked. Checking for carefully documented precondition failures and throwing IAE per clear documentation and with an informative message *at the point of method invocation* is much better, IMO[1].  I understand you disagree.  Lets leave things alone for now.

[1] From Eric Raymond's _The Art of Unix Programming_ "...when you must fail, fail noisily and as soon as possible"
http://catb.org/esr/writings/taoup/html/ch01s06.html#id2878538
               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Resolved] (MATH-856) Deprecate "NullArgumentException"

Dmitri Blinov (Jira)
In reply to this post by Dmitri Blinov (Jira)

     [ https://issues.apache.org/jira/browse/MATH-856?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gilles resolved MATH-856.
-------------------------

    Resolution: Won't Fix

This issue is superseded by MATH-869.
               

> Deprecate "NullArgumentException"
> ---------------------------------
>
>                 Key: MATH-856
>                 URL: https://issues.apache.org/jira/browse/MATH-856
>             Project: Commons Math
>          Issue Type: Task
>    Affects Versions: 3.0
>            Reporter: Gilles
>            Priority: Trivial
>             Fix For: 3.1, 4.0
>
>
> [Discussions|http://markmail.org/message/cl2e6c4pqbluo63e] on the "dev" ML concluded that "NullArgumentException" was more of a burden to maintain than it brings benefits.
> It will be deprecated in 3.1 and removed in 4.0.
> Checks for "null" in CM code will either be maintained or be removed.
> Whenever checks for "null" are performed, the exception to be thrown is the standard "NullPointerException" (instantiated with the no-arg constructor).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira