[jira] Created: (MATH-401) Policy concerning "null"

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

[jira] Created: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
Policy concerning "null"
------------------------

                 Key: MATH-401
                 URL: https://issues.apache.org/jira/browse/MATH-401
             Project: Commons Math
          Issue Type: Improvement
            Reporter: Gilles
            Assignee: Gilles
            Priority: Trivial
             Fix For: 3.0


Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-401) Policy concerning "null"

JIRA jira@apache.org

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

Phil Steitz commented on MATH-401:
----------------------------------

I am OK with removing the NPE wrapper but against the policy of never checking nulls.   In particlular, I am -1 on changing the StatUtils and PearsonsCorrelation classes to eliminate argument guards and jiust allow NPEs to be generated and thrown with no context info when one of the arguments in null.  I would like to retain the current behavior, where arguments are checked and an IAE with informative message is thrown when an argument is null.

There is value to checking arguments and throwing IAE when arguments not expected to be null are null.  This provides more information to the caller than just allowing an NPE to be encountered by implementation code and propagate.  

> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 3.0
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Gilles commented on MATH-401:
-----------------------------

The policy could be that IAE is thrown when encountering "null" references. Then, do you actually want to create all possible "informative" messages: "foo cannot be null" , "bar cannot be null", etc. (i.e. basically as many as there are types)?


> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 3.0
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Phil Steitz commented on MATH-401:
----------------------------------

Yes, these messages exist now and I would like to keep them.

> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 3.0
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Gilles commented on MATH-401:
-----------------------------

Some (less than 10) messages already exists. My question is: Do you plan to define one message per argument type used in CM (potentially in the hundreds)?


> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 3.0
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Phil Steitz commented on MATH-401:
----------------------------------

No, what is relevant is not "type" but argument in context.  I do plan to continue to add informative exception error messages - however we manage the strings.  Method parameter validation will result in different kinds of exception error messages, some indicating that required parameters (mentioned by formal parameter name, not type) expected not to be null are null; others indicating that arguments are out of range, etc.

> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 3.0
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (MATH-401) Policy concerning "null"

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Gilles resolved MATH-401.
-------------------------

    Fix Version/s: 2.2
                       (was: 3.0)
       Resolution: Fixed

Revision 982950 based on the consensus that a {{NullArgumentException}} (subclass of IAE) must be thrown when a reference is found to be unexpectedly {{null}}.


> Policy concerning "null"
> ------------------------
>
>                 Key: MATH-401
>                 URL: https://issues.apache.org/jira/browse/MATH-401
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Trivial
>             Fix For: 2.2
>
>
> Following the discussion on the "dev" ML, it appears that it would be fine to not check for {{null}} references within CM. The JVM will do it anyway so that it is redundant and not necessary most of the time (i.e. when usage is valid). When it happens, the problem is obvious enough that the standard {{NullPointerException}} fully describes it, without the need for additional specific and localized detailed messages.
> Hence we could remove all explicit checks for {{null}}. Or when early failure is preferred, make the check and throw {{NullPointerException}} (without a message argument).
> Since failed checks are currently reported with an {{IllegalArgumentException}}, the change is not backward-compatible.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.