[jira] Issue Comment Edited: (MATH-361) Localization and Error Handling

["Gilles (JIRA)" <ji...@apache.org>]

    [ 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.