Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: domain of
 SRS0=zeP9=HL=m4x.org=sebastien.brisard@bounces.m4x.org designates
 129.104.30.34 as permitted sender)
MIME-Version: 1.0
Date: Wed, 12 Sep 2012 13:05:34 +0200
Message-ID: 
 <CAGRH7HpCD-4OdOhsRk6PkLykp1WhEPFrXqruk+fzdkWUCJiONQ@mail.gmail.com>
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
From: =?ISO-8859-1?Q?S=E9bastien_Brisard?= <sebastien.brisard@m4x.org>
To: Commons Developers List <dev@commons.apache.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Hi Gilles,

2012/9/12 Gilles Sadowski <gilles@harfang.homelinux.org>:
> Hello S=E9bastien.
>
> 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=3D1383770&view=3Drev
>> Log:
>> Removed LocalizedFormats.ENTRY previously introduced in r1383747, as Loc=
alizedFormats.INDEX will do nicely.
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exc=
eption/util/LocalizedFormats.java
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/lin=
ear/ArrayFieldVector.java
>>     commons/proper/math/trunk/src/main/resources/assets/org/apache/commo=
ns/math3/exception/util/LocalizedFormats_fr.properties
>>     commons/proper/math/trunk/src/test/java/org/apache/commons/math3/exc=
eption/util/LocalizedFormatsTest.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/mat=
h3/exception/util/LocalizedFormats.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/jav=
a/org/apache/commons/math3/exception/util/LocalizedFormats.java?rev=3D13837=
70&r1=3D1383769&r2=3D1383770&view=3Ddiff
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exc=
eption/util/LocalizedFormats.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/exc=
eption/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 ch=
aracter"),
>>      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 co=
nfiguration"),
>>      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/mat=
h3/linear/ArrayFieldVector.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/jav=
a/org/apache/commons/math3/linear/ArrayFieldVector.java?rev=3D1383770&r1=3D=
1383769&r2=3D1383770&view=3Ddiff
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/lin=
ear/ArrayFieldVector.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/lin=
ear/ArrayFieldVector.java Wed Sep 12 04:43:38 2012
>> @@ -560,7 +560,7 @@ public class ArrayFieldVector<T extends
>>              try {
>>                  out[i] =3D one.divide(data[i]);
>>              } catch (final MathArithmeticException e) {
>> -                throw new MathArithmeticException(LocalizedFormats.ENTR=
Y, i);
>> +                throw new MathArithmeticException(LocalizedFormats.INDE=
X, 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) repo=
rt
> 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 t=
hat
> 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=E9bastien


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