Подтвердить что ты не робот

Почему считается ошибочной практикой определения метода ковариантного метода compareTo?

Вот пример из моего кода:

BaseClass:

abstract class AbstractBase implements Comparable<AbstractBase> {
    private int a;
    private int b;

    public int compareTo(AbstractBase other) {
        // compare using a and b
    }
}

Реализация:

class Impl extends AbstractBase {
private int c;

public int compareTo(Impl other) {
    // compare using a, b and c with c having higher impact than b in AbstractBase
}

FindBugs сообщает об этом как о проблеме. Но почему? Что может случиться?

И как правильно реализовать решение?

4b9b3361

ОТВЕТЫ

Ответ 1

Impl#compareTo(Impl) не переопределяет AbstractBase#compareTo(AbstractBase), поскольку они не имеют одинаковой сигнатуры. Другими словами, он не будет вызываться при использовании Collections#sort, например.

Ответ 2

EDIT: добавлено решение без каста

Если вы не хотите бросать, вы можете попробовать следующее.

Измените свой базовый класс на:

abstract class AbstractBase<T extends AbstractBase<?>> implements Comparable<T> {
//...
    public int compareTo(T other) {
      //... 
    }
}

И вам класс Impl для:

class Impl extends AbstractBase<Impl> {
//...
    @Override
    public int compareTo(Impl other) {
    //...
    }
}

Решение с литьем:

Возможным решением было бы переопределить метод compareTo (AbstractBase) в классе Impl и явно проверить, передан ли экземпляр Impl в:

   class Impl extends AbstractBase {
   //...
        @Override
        public int compareTo(AbstractBase other) {

            if (other instanceof Impl) {

                int compC = Integer.compare(c, ((Impl) other).c);

                if (compC == 0) {
                    return super.compareTo(other);
                }
                return compC;
            }

            return super.compareTo(other);
        }
    }

Ответ 3

Следующее - это то, что я пробовал. Не совсем уверен, что это причина, по которой findbugs выдает ошибку.

См. следующий код с гипотетической реализацией метода compareTo.

Сравнение одних и тех же объектов приводит к разным выходам.

public class Main
{
  public static void main(String[] args)
  {
    Impl implAssignedToImpl = new Impl(1, 2, 3);
    Impl otherImpl = new Impl(3, 2, 1);
    System.out.println(implAssignedToImpl.compareTo(otherImpl)); // prints -2

    AbstractBase implAssignedToAbstract = implAssignedToImpl;
    System.out.println(implAssignedToAbstract.compareTo(otherImpl)); //prints 0
  }
}

class AbstractBase implements Comparable<AbstractBase>
{
  private int a;

  private int b;

  public AbstractBase(int a, int b)
  {
    super();
    this.a = a;
    this.b = b;
  }

  public int compareTo(AbstractBase other)
  {
    return (a + b) - (other.a + other.b);
  }
}

class Impl extends AbstractBase
{
  private int c;

  public Impl(int a, int b, int c)
  {
    super(a, b);
    this.c = c;
  }

  public int compareTo(Impl other)
  {
    return super.compareTo(other) + (c - other.c);
  }
}

Основываясь на моей гипотетической compareTo, следующая, кажется, является хорошим решением. Вы можете попробовать иметь метод, похожий на getSum, который дает экземпляру объекта значение.

public class Main
{
  public static void main(String[] args)
  {
    Impl implAssignedToImpl = new Impl(1, 2, 3);
    Impl otherImpl = new Impl(3, 2, 1);
    System.out.println(implAssignedToImpl.compareTo(otherImpl)); // prints 0

    AbstractBase implAssignedToAbstract = implAssignedToImpl;
    System.out.println(implAssignedToAbstract.compareTo(otherImpl)); //prints 0
  }
}

class AbstractBase implements Comparable<AbstractBase>
{
  private int a;

  private int b;

  public AbstractBase(int a, int b)
  {
    super();
    this.a = a;
    this.b = b;
  }

  public int compareTo(AbstractBase other)
  {
    return getSum() - other.getSum();
  }

  public int getSum()
  {
    return a + b;
  }
}

class Impl extends AbstractBase
{
  private int c;

  public Impl(int a, int b, int c)
  {
    super(a, b);
    this.c = c;
  }

  @Override
  public int getSum()
  {
    return super.getSum() + c;
  }
}

Ответ 4

Как сказал sp00m, ваш Impl#compareTo(Impl) имеет другую подпись, чем AbstractBase#compareTo(AbstractBase), поэтому он не перегружает его.

Ключевым моментом является понимание того, почему он не работает, даже если вы пытаетесь сравнить sort() с другим Impl, где соответствует более конкретная подпись do.

Как вы определили Comparable<AbstractBase>, вам нужно определить, как ваш экземпляры compareTo AbstractBase экземпляров. И поэтому вам нужно реализовать compareTo(AbstractBase).

Вы можете думать, что, будучи Impl подтипом AbstractBase, более конкретный метод будет использоваться, когда происходит сравнение между двумя Impl. Проблема в Java имеет статическую привязку, поэтому компилятор определяет во время компиляции, какой метод будет использовать для решения каждого вызова метода. Если вы занимались сортировкой AbstractBase s, тогда компилятор использовал бы compareTo(AbstractBase), который определяет интерфейс AbstractBase, когда он реализует интерфейс Comparable(AbstractBase).

Вы можете использовать Impl интерфейс Comparable<Impl> для использования метода compareTo(Impl), но это будет работать, только если вы явно сортируете вещи, которые, как известно, Impl во время компиляции (т.е. Impl объект или Collection<Impl>).

Если вы действительно хотите применить другое сравнение, когда ваши два объекта Impl s, вы должны упасть до какой-то двойной отправки в вашем Impl#compareTo(AbstractBase), например:

Impl >>>
int compareTo(AbstractBase other) {
    return other.compareToImpl(this);
}

int compareToImpl(Impl other) {
    // perform custom comparison between Impl's
}

AbstractBase >>>
int compareTo(AbstractBase other) {
    // generic comparison
}

int compareToImpl(Impl other) {
    // comparison between an AbstractBase and an Impl.
    //Probably could just "return this.compareTo(other);", but check for loops :)
}

Это требует, чтобы вы добавили некоторую информацию Impl в свой AbstractBase, но это не очень красиво, но решает проблему более изящным способом, которым это может быть - использование отражения для этого - не элегантный на всех.

Ответ 5

Принцип подстановки Лискова (http://en.wikipedia.org/wiki/Liskov_substitution_principle) утверждает: если S является подтипом T, то объекты типа T могут быть заменены на объекты типа S (т.е. объекты типа S могут заменять объекты типа T) без изменения каких-либо желательных свойств этой программы (правильность, выполняемая задача и т.д.).

В вашем случае вы переопределяете метод compareTo из базового класса таким образом, чтобы нарушить поведение исходного метода. Вероятно, именно поэтому у FindBugs возникла проблема.

Если вы хотите быть соответствующим:

abstract class AbstractBase {
}

class Impl1 extends AbstractBase implements Comparable<Impl1> ...  
class Impl2 extends AbstractBase implements Comparable<Impl2> ...

ИЛИ

еще лучше, не используйте интерфейс Comparable вообще - вместо этого используйте Comparator во время сортировки.

Однако в реальной жизни есть ситуации, когда вам нужно обойти это (возможно, у вас нет доступа к источнику AbstractBase, или, может быть, ваш новый класс - это просто POC). В этих особых случаях я бы пошел с "уродливым" литым решением, предложенным Джоном.