You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Henri Yandell <fl...@gmail.com> on 2011/08/26 10:17:50 UTC

[lang] ComparatorChain redesign

Looking at the Collections ComparatorChain class, I think it's overcomplicated.

The current API is:

public class ComparatorChain<E> implements Comparator<E>, Serializable {
    public ComparatorChain() {
    public ComparatorChain(Comparator<E> comparator) {
    public ComparatorChain(Comparator<E> comparator, boolean reverse) {
    public ComparatorChain(List<Comparator<E>> list) {
    public ComparatorChain(List<Comparator<E>> list, BitSet bits) {
    public void addComparator(Comparator<E> comparator) {
    public void addComparator(Comparator<E> comparator, boolean reverse) {
    public void setComparator(int index, Comparator<E> comparator)
throws IndexOutOfBoundsException {
    public void setComparator(int index, Comparator<E> comparator,
boolean reverse) {
    public void setForwardSort(int index) {
    public void setReverseSort(int index) {
    public int size() {
    public boolean isLocked() {
    public int compare(E o1, E o2) throws UnsupportedOperationException {
    public int hashCode() {
    public boolean equals(Object object) {

The whole reverse notion seems like unnecessary duplication. If you
wanted such you could wrap a ReverseComparator around the comparator
before adding. I also think the notion of locking should also go away,
make these immutable objects.

ie:


public class ComparatorChain<E> implements Comparator<E>, Serializable {
    public ComparatorChain(Comparator<E> comparator...) {
    public ComparatorChain(List<Comparator<E>> list) {
    public int size() {
    public int compare(E o1, E o2) {
    public int hashCode() {
    public boolean equals(Object object) {

The constructors could throw exceptions when they contain no
comparator. Or we could simply return 0 as having no comparators is
the same as the comparators being exhausted.

Any thoughts on the simpler API?

Should it also implement Iterable as a way to loop through the
contained comparators?

Hen

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


Re: [lang] ComparatorChain redesign

Posted by James Ring <sj...@jdns.org>.
Hey Henri,

On Fri, Aug 26, 2011 at 11:50 PM, Henri Yandell <fl...@gmail.com> wrote:
> Thanks James.
>
> I wanted to pull the comparator classes over from Collections as I
> wanted to still be able to support them (and use internally as I
> already had ComparableComparator hidden away) without having to deal
> with all of Collections.
>
> Maybe the answer though is to ignore them in favour of Ordering, much
> the way we've long ignored the Date API in favour of Joda Time.
>
> It looks like:
>
> ComparableComparator = Ordering.natural
> ReverseComparator = Ordering.reverse
> NullComparator = Ordering.nullsFirst/nullsLast
> ComparatorChain = Ordering.from + Ordering.compound
> FixedOrderComparator = Ordering.explicit

I think one of the most useful methods is onResultOf, e.g.

class Person {
  public static final Function<Person, String> lastNameFunction = new
Function<Person, String>() {
    @Override public String apply(Person input) {
      return person.getLastName();
    }
  }

  ...
}

List<Person> people = ...;

Ordering<Person> lastNameOrdering =
Ordering.natural().reverse().onResultOf(lastNameFunction);
List<Person> sortedPeople = lastNameOrdering.immutableSortedCopy(people);

> Hen

Regards,
James

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


Re: [lang] ComparatorChain redesign

Posted by Henri Yandell <fl...@gmail.com>.
Thanks James.

I wanted to pull the comparator classes over from Collections as I
wanted to still be able to support them (and use internally as I
already had ComparableComparator hidden away) without having to deal
with all of Collections.

Maybe the answer though is to ignore them in favour of Ordering, much
the way we've long ignored the Date API in favour of Joda Time.

It looks like:

ComparableComparator = Ordering.natural
ReverseComparator = Ordering.reverse
NullComparator = Ordering.nullsFirst/nullsLast
ComparatorChain = Ordering.from + Ordering.compound
FixedOrderComparator = Ordering.explicit

Hen

On Fri, Aug 26, 2011 at 1:40 AM, James Ring <sj...@jdns.org> wrote:
> Google Guava has Ordering:
>
> http://guava-libraries.googlecode.com/svn/trunk/javadoc/com/google/common/collect/Ordering.html
>
> It is an incredibly useful class. Some random example of how it's awesome:
>
> http://www.polygenelubricants.com/2010/10/elegant-comparison-logic-with-guava.html
>
> Recommend that ComparatorChain.. just do what Ordering does. :)
>
> On Fri, Aug 26, 2011 at 1:17 AM, Henri Yandell <fl...@gmail.com> wrote:
>> Looking at the Collections ComparatorChain class, I think it's overcomplicated.
>>
>> The current API is:
>>
>> public class ComparatorChain<E> implements Comparator<E>, Serializable {
>>    public ComparatorChain() {
>>    public ComparatorChain(Comparator<E> comparator) {
>>    public ComparatorChain(Comparator<E> comparator, boolean reverse) {
>>    public ComparatorChain(List<Comparator<E>> list) {
>>    public ComparatorChain(List<Comparator<E>> list, BitSet bits) {
>>    public void addComparator(Comparator<E> comparator) {
>>    public void addComparator(Comparator<E> comparator, boolean reverse) {
>>    public void setComparator(int index, Comparator<E> comparator)
>> throws IndexOutOfBoundsException {
>>    public void setComparator(int index, Comparator<E> comparator,
>> boolean reverse) {
>>    public void setForwardSort(int index) {
>>    public void setReverseSort(int index) {
>>    public int size() {
>>    public boolean isLocked() {
>>    public int compare(E o1, E o2) throws UnsupportedOperationException {
>>    public int hashCode() {
>>    public boolean equals(Object object) {
>>
>> The whole reverse notion seems like unnecessary duplication. If you
>> wanted such you could wrap a ReverseComparator around the comparator
>> before adding. I also think the notion of locking should also go away,
>> make these immutable objects.
>>
>> ie:
>>
>>
>> public class ComparatorChain<E> implements Comparator<E>, Serializable {
>>    public ComparatorChain(Comparator<E> comparator...) {
>>    public ComparatorChain(List<Comparator<E>> list) {
>>    public int size() {
>>    public int compare(E o1, E o2) {
>>    public int hashCode() {
>>    public boolean equals(Object object) {
>>
>> The constructors could throw exceptions when they contain no
>> comparator. Or we could simply return 0 as having no comparators is
>> the same as the comparators being exhausted.
>>
>> Any thoughts on the simpler API?
>>
>> Should it also implement Iterable as a way to loop through the
>> contained comparators?
>>
>> Hen
>>
>> ---------------------------------------------------------------------
>> 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: [lang] ComparatorChain redesign

Posted by James Ring <sj...@jdns.org>.
Google Guava has Ordering:

http://guava-libraries.googlecode.com/svn/trunk/javadoc/com/google/common/collect/Ordering.html

It is an incredibly useful class. Some random example of how it's awesome:

http://www.polygenelubricants.com/2010/10/elegant-comparison-logic-with-guava.html

Recommend that ComparatorChain.. just do what Ordering does. :)

On Fri, Aug 26, 2011 at 1:17 AM, Henri Yandell <fl...@gmail.com> wrote:
> Looking at the Collections ComparatorChain class, I think it's overcomplicated.
>
> The current API is:
>
> public class ComparatorChain<E> implements Comparator<E>, Serializable {
>    public ComparatorChain() {
>    public ComparatorChain(Comparator<E> comparator) {
>    public ComparatorChain(Comparator<E> comparator, boolean reverse) {
>    public ComparatorChain(List<Comparator<E>> list) {
>    public ComparatorChain(List<Comparator<E>> list, BitSet bits) {
>    public void addComparator(Comparator<E> comparator) {
>    public void addComparator(Comparator<E> comparator, boolean reverse) {
>    public void setComparator(int index, Comparator<E> comparator)
> throws IndexOutOfBoundsException {
>    public void setComparator(int index, Comparator<E> comparator,
> boolean reverse) {
>    public void setForwardSort(int index) {
>    public void setReverseSort(int index) {
>    public int size() {
>    public boolean isLocked() {
>    public int compare(E o1, E o2) throws UnsupportedOperationException {
>    public int hashCode() {
>    public boolean equals(Object object) {
>
> The whole reverse notion seems like unnecessary duplication. If you
> wanted such you could wrap a ReverseComparator around the comparator
> before adding. I also think the notion of locking should also go away,
> make these immutable objects.
>
> ie:
>
>
> public class ComparatorChain<E> implements Comparator<E>, Serializable {
>    public ComparatorChain(Comparator<E> comparator...) {
>    public ComparatorChain(List<Comparator<E>> list) {
>    public int size() {
>    public int compare(E o1, E o2) {
>    public int hashCode() {
>    public boolean equals(Object object) {
>
> The constructors could throw exceptions when they contain no
> comparator. Or we could simply return 0 as having no comparators is
> the same as the comparators being exhausted.
>
> Any thoughts on the simpler API?
>
> Should it also implement Iterable as a way to loop through the
> contained comparators?
>
> Hen
>
> ---------------------------------------------------------------------
> 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