You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2011/05/20 22:04:07 UTC

[math] BigFraction add, subtract returning references rather than new instances

This code in BigFraction.add looks dangerous to me:

 if (ZERO.equals(fraction)) {
            return this;
 }

subtract has similar code and some other methods return the static
BigFraction.ZERO.

While BigFractions are Immutable, this could cause problems for
applications that are expecting new instances resulting from
arithmetic operations.  Can anyone see any reason that this should
not be changed to consistently create new instances?

Phil

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


Re: [math] BigFraction add, subtract returning references rather than new instances

Posted by Phil Steitz <ph...@gmail.com>.
On 5/21/11 1:50 AM, Luc Maisonobe wrote:
> Le 21/05/2011 01:25, sebb a écrit :
>> On 20 May 2011 21:04, Phil Steitz<ph...@gmail.com>  wrote:
>>> This code in BigFraction.add looks dangerous to me:
>>>
>>>   if (ZERO.equals(fraction)) {
>>>             return this;
>>>   }
>>>
>>> subtract has similar code and some other methods return the static
>>> BigFraction.ZERO.
>>>
>>> While BigFractions are Immutable, this could cause problems for
>>> applications that are expecting new instances resulting from
>>> arithmetic operations.  Can anyone see any reason that this should
>>> not be changed to consistently create new instances?
>
> Speed and memory. This is especially important for instances that
> are really used a lot (0, 1 ...) as building them anew a lot of
> times seems unneeded for immutable instances.
>
>>
>> Seems to me that an application that depends on getting new
>> instances is broken.
>
> I tend to agree here, but there is certainly a reason for such a
> need. Could you explain why new instances are mandatory ?

Could be there is no problem, it just seemed odd and potentially
dangerous to me that most activations produced new instances, but
some returned *this.  Sebb's pointing out that this "feature" is
shared by the jvm and thinking carefully through the use cases,
immutability and how equals is implemented I guess it does not make
any difference and does not really require separate documentation. 
I was thinking there could be instances where applications wanted to
store references to sets of results and somehow crossing pointer
references would lead to unexpected results or gc problems; but
thinking carefully through all of the examples I could come up with,
I can't see a problem, so I guess it is OK to leave as is.

Phil
>
> Luc
>
>>
>> Cf autoboxing which uses valueOf() which may return a cached
>> instance.
>>
>>> Phil
>>>
>>> ---------------------------------------------------------------------
>>>
>>> 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
>>
>>
>
>
> ---------------------------------------------------------------------
> 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


Re: [math] BigFraction add, subtract returning references rather than new instances

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Sat, May 21, 2011 at 10:50:01AM +0200, Luc Maisonobe wrote:
> Le 21/05/2011 01:25, sebb a écrit :
> >On 20 May 2011 21:04, Phil Steitz<ph...@gmail.com>  wrote:
> >>This code in BigFraction.add looks dangerous to me:
> >>
> >>  if (ZERO.equals(fraction)) {
> >>            return this;
> >>  }
> >>
> >>subtract has similar code and some other methods return the static
> >>BigFraction.ZERO.
> >>
> >>While BigFractions are Immutable, this could cause problems for
> >>applications that are expecting new instances resulting from
> >>arithmetic operations.  Can anyone see any reason that this should
> >>not be changed to consistently create new instances?
> 
> Speed and memory. This is especially important for instances that
> are really used a lot (0, 1 ...) as building them anew a lot of
> times seems unneeded for immutable instances.
> 
> >
> >Seems to me that an application that depends on getting new instances is broken.
> 
> I tend to agree here, but there is certainly a reason for such a
> need. Could you explain why new instances are mandatory ?

I think that sebb meant that there should _not_ be such a need (cf. his
statement below).

> >Cf autoboxing which uses valueOf() which may return a cached instance.

Regards,
Gilles

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


Re: [math] BigFraction add, subtract returning references rather than new instances

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 21/05/2011 01:25, sebb a écrit :
> On 20 May 2011 21:04, Phil Steitz<ph...@gmail.com>  wrote:
>> This code in BigFraction.add looks dangerous to me:
>>
>>   if (ZERO.equals(fraction)) {
>>             return this;
>>   }
>>
>> subtract has similar code and some other methods return the static
>> BigFraction.ZERO.
>>
>> While BigFractions are Immutable, this could cause problems for
>> applications that are expecting new instances resulting from
>> arithmetic operations.  Can anyone see any reason that this should
>> not be changed to consistently create new instances?

Speed and memory. This is especially important for instances that are 
really used a lot (0, 1 ...) as building them anew a lot of times seems 
unneeded for immutable instances.

>
> Seems to me that an application that depends on getting new instances is broken.

I tend to agree here, but there is certainly a reason for such a need. 
Could you explain why new instances are mandatory ?

Luc

>
> Cf autoboxing which uses valueOf() which may return a cached instance.
>
>> Phil
>>
>> ---------------------------------------------------------------------
>> 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
>
>


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


Re: [math] BigFraction add, subtract returning references rather than new instances

Posted by sebb <se...@gmail.com>.
On 20 May 2011 21:04, Phil Steitz <ph...@gmail.com> wrote:
> This code in BigFraction.add looks dangerous to me:
>
>  if (ZERO.equals(fraction)) {
>            return this;
>  }
>
> subtract has similar code and some other methods return the static
> BigFraction.ZERO.
>
> While BigFractions are Immutable, this could cause problems for
> applications that are expecting new instances resulting from
> arithmetic operations.  Can anyone see any reason that this should
> not be changed to consistently create new instances?

Seems to me that an application that depends on getting new instances is broken.

Cf autoboxing which uses valueOf() which may return a cached instance.

> Phil
>
> ---------------------------------------------------------------------
> 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