[
https://issues.apache.org/jira/browse/MATH-1041?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13796613#comment-13796613
]
Gilles commented on MATH-1041:
------------------------------
bq. But here I meant that within one issue you had me invite discussion to
iterate on the patch but had twice committed part of it before those changes
completed.
I indicated (see my first comment) that I could handle the modifications by
myself; hence I started as soon as Phil indicated he was OK with the
suggestions.
So let's work together in an orderly fashion. ;)
# IMO, the changes for "Pair.java" are completed: I think that the additional
tests are not necessary as they duplicate what the constructor's tests ought to
do.
# Comparator
## For clarity, the comparator name should be changed to
"PairLexicographicComparator".
## All fields and methods must be commented:
### "compare" method. What you wrote in the class comment could be moved
there; and in the class comment, you could put a '@link' to the "compare"
method.
### field "nullsFirst" (to be renamed also, see below).
## "nullsFirst" could better be renamed either "nullIsFirst" or "nullFirst".
> Add Pair factory method, toString(), Comparator
> ------------------------------------------------
>
> Key: MATH-1041
> URL: https://issues.apache.org/jira/browse/MATH-1041
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.2
> Reporter: Sean Owen
> Priority: Minor
> Labels: comparator, pair
> Attachments: MATH-1041_Comparator.patch, MATH-1041_Pair.patch
>
>
> I use Commons Math heavily, and have adopted its Pair class for the cases
> where I need, well, a pair of things.
> The attached patch adds three small improvements to the Pair class:
> - toString() method
> - factory method ".create()" to avoid duplicating generic types on instance
> creation
> - a Comparator
> Tests are included. I won't feel offended if this is rejected or modified but
> just wanted to supply the suggestion.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
