On Wed, Oct 16, 2013 at 1:13 AM, Phil Steitz <phil.ste...@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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
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 think the Comparator can also replace a
custom one in MathArrays.

Sean

Reply via email to