commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luc Maisonobe <Luc.Maison...@free.fr>
Subject Re: [math] Re: svn commit: r1078734
Date Wed, 09 Mar 2011 08:43:05 GMT
Le 08/03/2011 22:22, Gilles Sadowski a écrit :
> Hello.

Hi Gilles,

> 
>>> Modified: commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java?rev=1078734&r1=1078733&r2=1078734&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
(original)
>>> +++ commons/proper/math/trunk/src/test/java/org/apache/commons/math/ode/sampling/DummyStepInterpolatorTest.java
Mon Mar  7 10:37:52 2011
>>> @@ -123,7 +123,9 @@ public class DummyStepInterpolatorTest {
>>>          fail("an exception should have been thrown");
>>>      } catch (IOException ioe) {
>>>          // expected behavior
>>> -        assertEquals(0, ioe.getMessage().length());
>>> +        // XXX Why was the message supposed to be empty?
>>> +        // With the current code it is "org.apache.commons.math.util.Pair".
>>> +        // assertEquals(0, ioe.getMessage().length());
>>
>> The problem here is that the exception thrown which is an IOException
>> built from the empty string in the BadStepInterpolator below is not the
>> IOException that is caught here. In fact the IOException caught is
>> really a java.io.NotSerializableException which is built with the
>> message "org.apache.commons.math.util.Pair" automatically by the
>> serialization process in the JVM (at least for Oracle implementation).
>>
>> All exceptions must be serializable (this comes from java.lang.Throwable
>> implementing the Serializable interface). This was also one reason why
>> our Localizable interface had to extends Serializable, so it could be
>> used as a field in our exceptions.
>>
>> This change added a List<Pair<Localizable, Object[]>> field in
>> MathRuntimeException. Pair is not serializable. Adding Serializable to
>> the implemented interfaces in Pair does solve the problem (if also the
>> change below is reverted, of course) but I don't think it would be wise
>> unless we also enforce the two generic parameters of Pair to be
>> serializable too. Perhaps we should use a Map<Localizable, Object[]>
>> instead ?
> 
> No, because a key is unique (so that it won't be possible to add 2 messages
> with the same pattern but different values.
> I've created a "SerializablePair" (cf. revision 1079545) that seems to solve
> the problem.

Fine, thanks.

> 
>> The problem also makes me think that we already had a similar bug in
>> another part of the exceptions for a long time : the Object or Object[]
>> parameters of the exceptions are stored and may not be Serializable too.
>> This was never a problem either because the parameters are often simple
>> primitive ones (int, double ...) or arrays so they were serializable.
>> Perhaps we should change the signature from Object and Object[] to
>> Serializable and Serializable[] ?
> 
> I'm not fond of imposing "Serializable", as this would forbid user classes
> that are not "Serializable" (or force them to make the class "Serializable"
> although they don't have any use of that interface).
> I think "Object" is more flexible in the sense that users who need
> serialization will use "Serializable" classes and it will work...
> 
>>>      }
>>>  
>>>    }
>>> @@ -137,7 +139,7 @@ public class DummyStepInterpolatorTest {
>>>        }
>>>        @Override
>>>        protected void doFinalize() throws MathUserException {
>>> -          throw new MathUserException((Localizable) null, LocalizedFormats.SIMPLE_MESSAGE,
"");
>>> +          throw new MathUserException(LocalizedFormats.SIMPLE_MESSAGE, null);
>>
>> This should not have been replaced. Null is not the smae thing as the
>> empty String. Also this introduces a warning due to ambiguous signatures.
> 
> That was a mistake, but I didn't make it because I thought that
>   "" == null
> was true ;-}.

Ok, thanks

best regards,
Luc

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


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


Mime
View raw message