commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (MATH-361) Localization and Error Handling
Date Thu, 24 Jun 2010 14:22:49 GMT

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

Gilles edited comment on MATH-361 at 6/24/10 10:22 AM:
-------------------------------------------------------

I've created an "exception" package in r957518.

Currently, it contains 4 classes:
* {{MessageFactory}}: localized message formatting
* {{MathIllegalArgumentException}}: base class for exceptions that must inherit from the standard
{{IllegalArgumentException}} [Examples are shown in the last two classes.]
* {{DimensionMismatchException}}
* {{OutOfRangeException}}

Starting from this, I'd continue to create new exception classes as needed. The first step
would be to hunt down all the precondition violation checks and replace the "MathRuntimeException.createIllegalArgumentException"
calls by an instantiation of the appropriate subclass of the new  {{MathIllegalArgumentException}}
class. I stress that this should lead to a reduction of the number of elements in the {{LocalizedFormats}}
{{enum}}. Indeed, take this code for example:

{code}
/**
 * Modify the shape parameter, alpha.
 * @param newAlpha the new shape parameter.
 * @throws IllegalArgumentException if <code>newAlpha</code> is not positive.
 */
private void setAlphaInternal(double newAlpha) {
    if (newAlpha <= 0.0) {
        throw MathRuntimeException.createIllegalArgumentException(
              LocalizedFormats.NOT_POSITIVE_ALPHA,
              newAlpha);
    }
    this.alpha = newAlpha;
}
{code}

What advantage does one get by having a specific "NOT_POSITIVE_ALPHA"? This is redundant with
the Javadoc. The actual error is that the argument is not strictly positive. The exception
thrown must reflect that, and _only_ that. The user _must_ read the Javadoc (as stated in
the user guide) in order to figure out why this error has occurred. Agreeing on this issue
will enable the replacement of many message patterns by their common "error" description.
If you don't accept this simplification there can be no end in the {{enum}} list, and it will
become increasingly difficult to reuse existing elements as the new cases will almost always
be different (even if only slightly so), thus requiring yet another element!

Another point is that the old  {{DimensionMismatchException}} is a checked exception. I think
that it was an unfortunate choice and the required change is not backward-compatible. Still
IMO the change should be performed before release 3.0. In the meantime, is there a way to
warn users that this change will occur?


      was (Author: erans):
    I've created an "exception" package in r957518.

Currently, it contains 4 classes:
* {{MessageFactory}}: localized message formatting
* {{MathIllegalArgumentException}}: base class for exceptions that must inherit from the standard
{{IllegalArgumentException}} [Examples are the shown in the last two classes.]
* {{DimensionMismatchException}}
* {{OutOfRangeException}}

Starting from this, I'd continue to create new exception classes as needed. The first step
would be to hunt down all the precondition violation checks and replace the "MathRuntimeException.createIllegalArgumentException"
calls by an instantiation of the appropriate subclass of the new  {{MathIllegalArgumentException}}
class. I stress that this should lead to a reduction of the number of elements in the {{LocalizedFormats}}
{{enum}}. Indeed, take this code for example:

{code}
/**
 * Modify the shape parameter, alpha.
 * @param newAlpha the new shape parameter.
 * @throws IllegalArgumentException if <code>newAlpha</code> is not positive.
 */
private void setAlphaInternal(double newAlpha) {
    if (newAlpha <= 0.0) {
        throw MathRuntimeException.createIllegalArgumentException(
              LocalizedFormats.NOT_POSITIVE_ALPHA,
              newAlpha);
    }
    this.alpha = newAlpha;
}
{code}

What advantage does one get by having a specific "NOT_POSITIVE_ALPHA"? This is redundant with
the Javadoc. The actual error is that the argument is not strictly positive. The exception
thrown must reflect that, and _only_ that. The user _must_ read the Javadoc (as stated in
the user guide) in order to figure out why this error has occurred. Agreeing on this issue
will enable the replacement of many message patterns by their common "error" description.
If you don't accept this simplification there can be no end in the {{enum}} list, and it will
become increasingly difficult to reuse existing elements as the new cases will almost always
be different (even if only slightly so), thus requiring yet another element!

Another point is that the old  {{DimensionMismatchException}} is a checked exception. I think
that it was an unfortunate choice and the required change is not backward-compatible. Still
IMO the change should be performed before release 3.0. In the meantime, is there a way to
warn users that this change will occur?

  
> Localization and Error Handling
> -------------------------------
>
>                 Key: MATH-361
>                 URL: https://issues.apache.org/jira/browse/MATH-361
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.1
>            Reporter: Gilles
>            Priority: Minor
>         Attachments: l10n.tar.gz, res.tar.gz
>
>
> This proposal aims at easing the handling of error during algorithms development, and
also enhancing the flexibility of the error reporting (provide meaningful exception classes
and run-time selection of the localization formatting).
> More details at [http://www.mail-archive.com/dev@commons.apache.org/msg14570.html]

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


Mime
View raw message