commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sébastien Brisard <sebastien.bris...@m4x.org>
Subject Re: svn commit: r1383770 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/exception/util/ main/java/org/apache/commons/math3/linear/ main/resources/assets/org/apache/commons/math3/exception/util/ test/java/org/apache/commons/ma
Date Wed, 12 Sep 2012 11:05:34 GMT
Hi Gilles,

2012/9/12 Gilles Sadowski <gilles@harfang.homelinux.org>:
> Hello Sébastien.
>
> On Wed, Sep 12, 2012 at 04:43:39AM -0000, celestin@apache.org wrote:
>> Author: celestin
>> Date: Wed Sep 12 04:43:38 2012
>> New Revision: 1383770
>>
>> URL: http://svn.apache.org/viewvc?rev=1383770&view=rev
>> Log:
>> Removed LocalizedFormats.ENTRY previously introduced in r1383747, as LocalizedFormats.INDEX
will do nicely.
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exception/util/LocalizedFormats.java
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/ArrayFieldVector.java
>>     commons/proper/math/trunk/src/main/resources/assets/org/apache/commons/math3/exception/util/LocalizedFormats_fr.properties
>>     commons/proper/math/trunk/src/test/java/org/apache/commons/math3/exception/util/LocalizedFormatsTest.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exception/util/LocalizedFormats.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exception/util/LocalizedFormats.java?rev=1383770&r1=1383769&r2=1383770&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exception/util/LocalizedFormats.java
(original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exception/util/LocalizedFormats.java
Wed Sep 12 04:43:38 2012
>> @@ -97,7 +97,6 @@ public enum LocalizedFormats implements
>>      EMPTY_SELECTED_ROW_INDEX_ARRAY("empty selected row index array"),
>>      EMPTY_STRING_FOR_IMAGINARY_CHARACTER("empty string for imaginary character"),
>>      ENDPOINTS_NOT_AN_INTERVAL("endpoints do not specify an interval: [{0}, {1}]"),
>> -    ENTRY("entry {0}"),
>>      EQUAL_VERTICES_IN_SIMPLEX("equal vertices {0} and {1} in simplex configuration"),
>>      EULER_ANGLES_SINGULARITY("Euler angles singularity"),
>>      EVALUATION("evaluation"), /* keep */
>
> Please discard my previous message...
> Again one disability of mine (apart from not being an HTML parser): I
> process messages sequentially. :-) Sorry.
>
As for me, I can't read emails (you pointed at INDEX in one of your
previous messages...).

>
> But, there was something else:
>
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/ArrayFieldVector.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/ArrayFieldVector.java?rev=1383770&r1=1383769&r2=1383770&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/ArrayFieldVector.java
(original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/ArrayFieldVector.java
Wed Sep 12 04:43:38 2012
>> @@ -560,7 +560,7 @@ public class ArrayFieldVector<T extends
>>              try {
>>                  out[i] = one.divide(data[i]);
>>              } catch (final MathArithmeticException e) {
>> -                throw new MathArithmeticException(LocalizedFormats.ENTRY, i);
>> +                throw new MathArithmeticException(LocalizedFormats.INDEX, i);
>>              }
>>          }
>
> Do we really want to do this instead of checking a precondition (or do
> nothin at that level)? I know that it would be preferrable to (also) report
> the INDEX, but on the other hand, this kind of code becomes really ugly (in
> the sense that there are more signs related to failure detection and
> handling than to the "interesting stuff".
>
I agree that it is ugly. I would like to know what others think. I'm
perfectly happy propagating a MathArithmeticException which does not
report on the index.

> Moreover, you can an exception and completely discard the information it
> might have contained! [In this case, the info might likely have been
> "division by zero".]
>
> Finally a more general point: In the "FieldElement" interface, there is
> ---
>     /** Compute this &divide; a.
>      * @param a element to add
>      * @return a new element representing this &divide; a
>      * @throws NullArgumentException if {@code a} is {@code null}.
>      * @throws MathArithmeticException if {@code a} is zero
>      */
>     T divide(T a) throws NullArgumentException, MathArithmeticException;
> ---
>
> This is another example of what I pointed to a few days ago: Documenting
> MathArithmeticException is wrong because not all implementations behave that
> way (e.g. "Complex"). [Alternatively, it can be construed that "Complex" is
> not correctly implemented (cf. MATH-667).]
>
> [There is also a typo in the description of param "a".]
>
> Best regards,
> Gilles
>
For what it's worth, Decimal64 does not throw an exception either. So
maybe MathArithmeticException should be removed from the signature of
FieldElement.divide(), as it's clearly not part of the contract of
this method.
However, some fields do not know NaN, and it would be nice to have a
guidance in the javadoc of the interface, regarding what exception
should be thrown.
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message