commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [math] Add Pair factory method, toString(), Comparator
Date Wed, 16 Oct 2013 08:52:32 GMT
On Wed, 16 Oct 2013 09:28:54 +0100, Sean Owen wrote:
> On Wed, Oct 16, 2013 at 1:13 AM, Phil Steitz <phil.steitz@gmail.com> 
> wrote:
>
>> On 10/15/13 2:51 PM, Sean Owen wrote:
>> > Hello all,
>> >
>> > I'd like to propose a few small additions to the Pair class in 
>> Common
>> > Math3: a factory method, to avoid redundant generics-related
>> eclarations, a
>> > toString() method, and a basic Comparator.
>> >
>> > It's already pretty well summarized, simple as it is, at
>> > https://issues.apache.org/jira/browse/MATH-1041 along with a 
>> current
>> patch.
>> >
>> > Thanks for your feedback.
>> >
>> > Sean
>> >
>>
>> Seems useful.  The factory and toString I see as nice additions.
>> The Comparator I am not so sure about as it only works for
>> Comparables.  If we do add it, It should probably have a more
>> descriptive name like LexicographicPairComparator, since it is
>> implementing just one way to order pairs.  It might be best also to
>> make the null behavior configurable - throw, null high, null low.  I
>> guess low or high is forced if we want it to be consistent with
>> equals, given the way Pair implements equals, so maybe just an
>> optional boolean config.  I am OK leaving as is with clear javadoc
>> (which the patch has, modulo fixing s/of/create in the example).  I
>> am OK with either of or create as the factory method name.
>>
>> Thanks for the patch!
>>
>> Phil
>>
>>
> All done in an updated patch in MATH-1041.
>
> I had originally used .of() for consistency with Commons Lang (is 
> that
> persuasive?) but edited at Gilles's request. The javadoc is fixed to 
> be
> consistent now.
>
> Would it be of interest to supply another patch changing calling code 
> to
> use the factory method? It has no functional change, just might make 
> the
> callers a tiny bit less verbose.

I don't think this is necessary at this point.

> I think the Comparator can also replace a
> custom one in MathArrays.

Are you sure?


Regards,
Gilles


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


Mime
View raw message