Рефакторинг универсального метода compareTo

Question

Что я делаю:

У меня есть контейнерный класс с именем Os, который может содержать элементы разных типов, а также экземпляры класса Os. Когда я сравниваю этот класс, я хочу видеть:

• мелкие равные для элементов
• Глубина равна для Os элементов

Я гарантировал, что каждый отдельный элемент, содержащийся в классе:

• Не может быть нулевым.
• Сравним с элементами того же типа.
• Неизменен. Ну, по крайней мере, часть, которую я проверяю.

Следующее - это то, что у меня есть в данный момент.

* +1025 * Пример:

Например, этот контрольный пример будет пройден.

    Os o1 = Os.of(3, 4d, Os.of("-"));
    Os o2 = Os.of(Os.of(Character.toString('-')), 4.0, new Integer(3));

    assertEquals(o1.toString(), "[3, 4.0, [-]]");
    assertEquals(o2.toString(), "[[-], 4.0, 3]");
    assertTrue(o1.reverse().compareTo(o2) == 0);

Пример кода:

compareTo метод:

@Override
public int compareTo(final Os that) {
    final int BEFORE = -1;
    final int EQUAL = 0;
    final int AFTER = 1;
    int subresult = 0;
    Comparable<?> othis;
    Comparable<?> othat;

    if (that == null)
        return AFTER;
    if (this == that)
        return EQUAL;

    subresult = ((Integer) this.o.size()).compareTo(that.o.size());
    if (subresult < 0)
        return BEFORE;
    else if (subresult > 0)
        return AFTER;

    try {
        for (int i = 0; i < this.o.size(); i++) {
            othis = this.o.get(i);
            othat = that.o.get(i);

            if (othis.getClass() == othat.getClass()) {
                if (othat instanceof Os) {
                    subresult = ((Os) othis).compareTo(((Os) othat));
                    if (subresult < 0)
                        return BEFORE;
                    else if (subresult > 0)
                        return AFTER;
                } else {
                    subresult = hackCMP(othis, othat);
                    if (subresult < 0)
                        return BEFORE;
                    else if (subresult > 0)
                        return AFTER;
                }
            } else {
                subresult = othis.getClass().getName()
                        .compareTo(othat.getClass().getName());
                if (subresult < 0)
                    return BEFORE;
                else if (subresult > 0)
                    return AFTER;
            }
        }
        return EQUAL;
    } catch (SecurityException e) {
        e.printStackTrace();
    } catch (IllegalArgumentException e) {
        e.printStackTrace();
    } catch (NoSuchMethodException e) {
        e.printStackTrace();
    } catch (IllegalAccessException e) {
        e.printStackTrace();
    } catch (InvocationTargetException e) {
        e.printStackTrace();
    }
    return BEFORE;
}

private static int hackCMP(Object val, Object val2)
        throws SecurityException, NoSuchMethodException,
        IllegalArgumentException, IllegalAccessException,
        InvocationTargetException {
    Method m = val.getClass().getMethod("compareTo", val.getClass());
    return (Integer) m.invoke(val, val2);
}

Вопрос:

Я хотел бы изменить код.

Например:

• Я бы предпочел не использовать метод hackCMP, если это возможно.

• Кажется, следующий фрагмент кода повторяется. Могу ли я заменить его чем-то?

          subresult = <expression>;
          if (subresult < 0)
              return BEFORE;
          else if (subresult > 0)
              return AFTER;
          //else ...
  

Что я могу сделать рефакторинг и как это сделать?

Edit:

@ wolfcastle: данные хранятся в private final ImmutableList<Comparable<?>> o;.

Я хотел бы отметить, что каждый ответ был полезен. Кажется, работает следующее:

@Override
public int compareTo(final Os that) {
    Ordering<Iterable<Comparable<?>>> order = //
    Ordering.natural().<Comparable<?>> lexicographical();
    int result = -1;

    try {
        result = ComparisonChain.start()
                .compare(this.o.size(), that.o.size())
                .compare(this.o, that.o, order).result();
    } catch (Exception e) { //ignore: type mismatch
    }

    return result;
}

Answer 1

Один из вариантов, который я бы рассмотрел, - это сохранение элементов в классе, который позволяет сравнивать их по классу, а не по методу compareTo, если они не принадлежат одному и тому же классу:

private static class Element implements Comparable<Element> {
  // raw Comparable allows you to call compareTo
  private final Comparable comparable;

  Element(Comparable comparable) {
    this.comparable = comparable;
  }

  @Override @SuppressWarnings("unchecked")
  public int compareTo(Element o) {
    Comparable other = o.comparable;
    if(comparable.getClass().isInstance(other)) {
      return comparable.compareTo(other);
    }

    return comparable.getClass().getName().compareTo(other.getClass().getName());
  }

  @Override
  public boolean equals(Object obj) {
    return obj instanceof Element && comparable.equals(((Element) obj).comparable);
  }

  @Override
  public int hashCode() {
    return comparable.hashCode();
  }

  @Override
  public String toString() {
    return comparable.toString();
  }
}

Тогда, когда ваш внутренний список является List<Element>, ваш compareTo метод в Os может быть довольно простым.Используя Guava , это может быть очень просто:

@Override
public int compareTo(Os o) {
  return ComparisonChain.start()
       .compare(list.size(), o.list.size())
       .compare(list, o.list, Ordering.natural().<Element>lexicographical())
       .result();
}

Answer 2

Поскольку универсальный тип свойства List<Comparable<?>> o не является фиксированным, я бы избавился от универсального типа и полагался на необработанный тип. Это стоит один @SuppressWarnings("rawtypes"), но это минимизирует много.

@Override
@SuppressWarnings("rawtypes")
public int compareTo(final Os that) {
    final int BEFORE = -1;
    final int EQUAL = 0;
    final int AFTER = 1;

    if (that == null)
        return AFTER;
    if (this == that)
        return EQUAL;

    int subresult = ((Integer) this.o.size()).compareTo(that.o.size());

    if (subresult != EQUAL)
        return subresult;

    for (int i = 0; i < this.o.size(); i++) {
        Comparable othis = this.o.get(i);
        Comparable othat = that.o.get(i);
        subresult = othis.compareTo(othat);

        if (subresult != EQUAL)
            return subresult;
    }

    return EQUAL;
}

Answer 3

У вас может быть метод, который возвращает ДО |ПОСЛЕ |INDETERMINATE (скажем), затем назовите его.

result = newMethod(subresult);
if (result != INDETERMINATE) return result;

Это не так уж много улучшений, и его все равно нужно дублировать везде, но оно немного теснее.