commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Henri Yandell <flame...@gmail.com>
Subject [lang] ComparatorChain redesign
Date Fri, 26 Aug 2011 08:17:50 GMT
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


Mime
View raw message