为什么定义一个协变的compareTo方法被认为是不良实践？

Question

这是我的代码示例：

基类：

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 报告了这个问题，但为什么会出现这种情况？可能会发生什么？

我该如何正确地实现解决方案？

- Uwe Allner

---

这不是协变。在 Impl 中，你正在重载（尝试在那里放置 @Override）。 - pingw33n

对于像我一样困惑的其他人，这里是FindBugs的链接(http://findbugs.sourceforge.net/bugDescriptions.html#CO_ABSTRACT_SELF)。显然，FindBugs认为compareTo应该始终采用一个Object。就个人而言，我不知道他们为什么这样认为。另外，正如指出的那样，这里还有另一个问题。 - Radiodef

但是你在 Impl 里面使用的 compareTo(Impl other)，是一个常见错误，它不会像预期那样运行。它没有重写 Comparable 接口的方法。 - Radiodef

问题是：你想在这里实现什么？考虑AbstractBase的子类A和B的实例a和b：即使它们来自不同的子类，你是否希望将a和b进行比较（即，你是否希望将AbstractBase的任何实例与其他任何实例进行比较）？或者你想指定每个AbstractBase的子类S需要能够将其自己的实例相互比较，但不一定需要将其自己的实例与其他子类的实例进行比较？ - Dirk

@UweAllner 您可以创建一个单独的 Comparator<Impl>，并在排序过程中使用它。 - Duncan Jones

Answer 1

Impl#compareTo(Impl)并不是覆盖了AbstractBase#compareTo(AbstractBase)，因为它们没有相同的签名。换句话说，例如使用Collections#sort时，它不会被调用。

- sp00m

---

但是它被调用了，因为sort不进行向下转换；如果集合声明为例如List<AbstractBase>，则可能不会被调用，而我没有这样做。 - Uwe Allner

你确定它被调用了吗？在 Impl#compareTo 中加入一个 syso。如果打印出来了，那么你提供的代码就不是你正在运行的代码，因为它只有在显式地调用时才能被调用（例如 new Impl().compareTo(...)）。sort 方法使用了 AbstractBase#compareTo 方法。 - sp00m

抱歉，你是对的，是我的错误。AbstractBase的compareTo被调用了。现在我想知道为什么，在排序中元素被转换为Comparable（未类型化），而比较的对象是一个Impl... - Uwe Allner

@UweAllner 实现一个简单的泛型集合并添加一个排序方法。然后你会发现，除了使用反射，这是唯一的方法。 - CodesInChaos

Answer 2

编辑：添加了不需要转换的解决方案。

如果您不想进行转换，可以尝试以下方法。

修改您的基类为：

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) {
    //...
    }
}

使用类型转换进行解决：

一个可能的解决方案是在Impl类中重写compareTo(AbstractBase)方法，并显式检查是否传入了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);
        }
    }

- John

---

我也考虑过这个问题，但是我认为这种强制转换相当丑陋。而且，FindBugs提出的使用Object作为compareTo方法的参数的解决方案更不用说了... - Uwe Allner

虽然看起来有些丑陋，但我会尝试使用转换解决方案；否则我还得修改所有派生对象。感谢你的努力。 - Uwe Allner

Answer 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;
  }
}

- Can't Tell

---

那种情况不会发生（当然，FindBugs无法知道）。但它很可能是消息的原因。 - Uwe Allner

@UweAllner 我编辑了答案并添加了解决方案。也许你可以尝试类似的方法？ - Can't Tell

我的例子有点简化了；实际上我还需要比较除整数以外的其他值；所以我会把签名问题转移到计算方法中。但无论如何，还是谢谢你的努力。 - Uwe Allner

Answer 4

正如sp00m所说，你的Impl#compareTo(Impl)与AbstractBase#compareTo(AbstractBase)的签名不同，因此它并没有重载它。

关键点在于理解为什么它不起作用，即使你尝试使用另一个Impl进行比较，更具体的签名确实匹配。

由于你定义了Comparable<AbstractBase>，因此你需要定义如何比较你的实例compareToAbstractBase实例。因此，你需要实现compareTo(AbstractBase)。

你可以认为，由于Impl是AbstractBase的子类型，当两个Impl之间进行比较时，会使用更具体的方法。问题在于Java具有静态绑定，因此编译器在编译时定义哪个方法用于解决每个方法调用。如果你正在对AbstractBase进行排序，那么编译器将使用AbstractBase的接口定义的compareTo(AbstractBase)，这是当它实现Comparable(AbstractBase)接口时定义的方法。

你可以让Impl实现Comparable<Impl>接口，以便使用compareTo(Impl)方法，但这只有在你在编译时明确排序已知为Impl的事物时才起作用（即一个Impl对象或Collection<Impl>）。

如果你真的想在两个对象都是Impl时应用不同的比较，那么你应该在你的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 :)
}

这需要您在AbstractBase中添加一些Impl信息，虽然不很美观，但解决了问题，并且是更加优雅的方法 - 使用反射来做这件事是不优雅的。

- mgarciaisaia

Answer 5

里氏替换原则（Liskov substitution principle）http://en.wikipedia.org/wiki/Liskov_substitution_principle指出：如果S是T的子类型，那么类型为T的对象可以被类型为S的对象所替代（即类型为S的对象可以代替类型为T的对象），而不会改变程序的任何可取属性（正确性、执行的任务等）。

在你的情况下，你重写了Base类中的compareTo方法，以一种破坏原始方法行为的方式。这可能是FindBugs有问题的原因。

如果您想做到正确，请参考以下内容：

abstract class AbstractBase {
}

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

或者更好的做法是，在排序时根本不使用Comparable接口 - 而是使用Comparator。

然而，在现实生活中有些情况下，你需要绕过它（也许你无法访问AbstractBase的源代码，或者你的新类只是一个POC）。在这些特殊情况下，我会选择John提出的“丑陋”的转换解决方案。

- firtydank