“instanceof”操作符的使用是否被认为是糟糕的设计？

Question

在我的一个项目中，我有两个“数据传输对象”RecordType1和RecordType2，它们都继承自RecordType的抽象类。

我希望两个RecordType对象都能在同一个RecordProcessor类中由“process”方法处理。我的第一想法是创建一个通用的process方法，将其委托给两个特定的process方法，如下所示：

public RecordType process(RecordType record){

    if (record instanceof RecordType1)
        return process((RecordType1) record);
    else if (record instanceof RecordType2)
        return process((RecordType2) record);

    throw new IllegalArgumentException(record);
}

public RecordType1 process(RecordType1 record){
    // Specific processing for Record Type 1
}

public RecordType2 process(RecordType2 record){
    // Specific processing for Record Type 2
}

我读过 Scott Meyers 在《Effective C++》中写的以下内容：

“每当你发现自己编写形如‘如果对象是类型 T1，那么做某事，但如果它是类型 T2，则做其他事情’的代码时，你就应该扇自己一个耳光。”

如果他是正确的，显然我应该扇自己一个耳光。 我真的不明白为什么这是不好的设计（除非当然有人对 RecordType 进行子类化并添加一个 RecordType3 而没有在处理它的通用“Process”方法中添加另一行，从而创建 NPE），而我能想到的替代方案涉及将特定处理逻辑的重点放在 RecordType 类本身中，这对我来说真的没有太多意义，因为理论上可以对这些记录执行许多不同类型的处理。

可以有人解释为什么这可能被认为是不良设计，并提供一些替代方案，仍然将处理这些记录的责任交给“Processing”类吗？

更新：

- 将 return null 更改为 throw new IllegalArgumentException(record); - 仅澄清一下，简单的 RecordType.process() 方法无法满足三个原因：首先，处理与 RecordType 相距甚远，不值得在 RecordType 子类中拥有自己的方法。 其次，有各种不同类型的处理器可以在理论上执行不同的处理。 最后， RecordType 被设计为一个简单的 DTO 类，其中仅定义了最小限度的状态更改方法。

- Eternal Rubyist

---

通常这是一个不好的迹象。但我们需要了解你想做什么 - 从问题中并不清楚。一般来说，似乎所有类型都具有相同的功能，因此您可以只使用接口。 - Yossale

RecordType1和2有什么区别？你能否让它们遵循相同的接口？ - kba

通常情况下，您不希望这样做，但如果您只看到该逻辑表达一次，也许您可以容忍它。如果您开始在多个地方看到相同的 if (a instanceof X) 逻辑表达式，那么您真的需要自我反省。 - Anthony Pegram

嗯，RecordType1和RecordType2有任何共同的子类吗？ - Joachim Isaksson

请注意，Scott Myers 的引用来自第一版的第39项“避免在继承层次结构中进行转换”-第138页。他明确表示，C++（约1992年）没有instanceof机制是一件好事。 - Bob Cross

Answer 1

这种情况通常使用Visitor模式。虽然代码会稍微复杂一些，但是在添加一个新的RecordType子类后，你必须在所有地方实现逻辑，否则代码将无法编译。到处都有instanceof非常容易遗漏一两个地方。

例子：

public abstract class RecordType {
    public abstract <T> T accept(RecordTypeVisitor<T> visitor);
}

public interface RecordTypeVisitor<T> {
    T visitOne(RecordType1 recordType);
    T visitTwo(RecordType2 recordType);
}

public class RecordType1 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitOne(this);
    }
}

public class RecordType2 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitTwo(this);
    }
}

使用方法（注意通用返回类型）：

String result = record.accept(new RecordTypeVisitor<String>() {

    String visitOne(RecordType1 recordType) {
        //processing of RecordType1
        return "Jeden";
    }

    String visitTwo(RecordType2 recordType) {
        //processing of RecordType2
        return "Dwa";
    }

});

我建议抛出异常：

throw new IllegalArgumentException(record);

当找不到任何类型时，不要返回null。

- Tomasz Nurkiewicz

---

这看起来很有前途...那么除了更复杂之外，使用这种模式的主要好处是什么？是否仅仅是因为一个人试图扩展它并忘记包含一个案例而无法打破框架？还有trzy和cztery在哪里（开个玩笑）？ - Eternal Rubyist

@nomizzz：是的，这种模式强制你在编译时实现子类特定的逻辑。此外，维基百科还带来了另一个好处，我之前不知道：“访问者对象可以具有状态”。我应该把你引导到GoF模式书中，而不是引用它；-）。是的，代码有点更复杂，甚至在某种程度上更难以阅读。 - Tomasz Nurkiewicz

这是一个非常好的Visitor实现。大多数人都会忘记泛型类型边界，从而失去强类型。 - Visionary Software Solutions

你可以使用重载，而不是 visitOne(RecordType1 r) 和 visitTwo(RecordType2 r)，来实现 visit(RecordType1 r) 和 visit(RecordType2 r)。 - chocalaca

Answer 2

我的建议：

public RecordType process(RecordType record){
    return record.process();
}

public class RecordType
{
    public RecordType process()
    {
        return null;
    }
}

public class RecordType1 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

public class RecordType2 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

如果您需要执行的代码与模型不应该知道的内容（如UI）耦合，那么您将需要使用某种双重调度或访问者模式。

http://en.wikipedia.org/wiki/Double_dispatch

- Ivo

---

是的，正如我在问题中所讨论的那样，我最初考虑了那个解决方案。但是它存在一些问题。首先，处理过程与RecordType相距太远，不值得在RecordType子类中单独定义一个方法。此外，理论上可以由不同的处理器执行各种不同类型的处理。最后，RecordType旨在成为一个简单的DTO类，其中仅定义了最小限度的状态更改方法。 - Eternal Rubyist

@ivowiblo 为什么不像OP建议的那样保持RecordType抽象，并将process()方法也设为抽象呢？ - matsev

因为我不知道它是抽象的。而且，由于示例代码在不是1或2的情况下返回null，所以我将其留给了基类。 - Ivo

@nomizzz，所以双重分派是最好的选择。尤其是考虑到您预计会有“大量不同类型的处理”时。 - Ivo

Answer 3

这违反了SOLID中的开闭原则

- k4dima

Answer 4

设计是达到目的的手段，如果不知道你的目标或限制，没有人能告诉你在特定情况下你的设计是否好，或者如何改进。

然而，在面向对象设计中，将方法实现保持在单独的类中，同时仍然为每种类型提供单独的实现的标准方法是访问者模式。

附注：在代码审查中，我会标记return null，因为它可能会传播错误而不是报告错误。请考虑：

RecordType processed = process(new RecordType3());

// many hours later, in a different part of the program

processed.getX(); // "Why is this null sometimes??"

换句话说，假定无法到达的代码路径应该抛出异常而不是导致未定义的行为。

- meriton

Answer 5

一个问题是设计不良，就像你的例子一样，在适用时没有使用访问者模式。

另一个问题是效率。与其他技术（如使用等式比较class对象）相比，instanceof非常慢。

当使用访问者模式时，通常一个有效而优雅的解决方案是使用Map来映射支持的class和Visitor实例。大型的if ... else块和instanceof检查会非常低效。

- Danubian Sailor

Answer 6

另一种可能的方法是将process()（或者称其为“doSubclassProcess()”以澄清事情）抽象化（在RecordType中），并在子类中实际实现。例如：

class RecordType {
   protected abstract RecordType doSubclassProcess(RecordType rt);

   public process(RecordType rt) {
     // you can do any prelim or common processing here
     // ...

     // now do subclass specific stuff...
     return doSubclassProcess(rt);
   }
}

class RecordType1 extends RecordType {
   protected RecordType1 doSubclassProcess(RecordType RT) {
      // need a cast, but you are pretty sure it is safe here
      RecordType1 rt1 = (RecordType1) rt;
      // now do what you want to rt
      return rt1;
   }
}

注意一下几个错别字 - 我想我已经修复了它们。

- user949300

---

通常来说，组合/接口优于继承，但继承仍是一个不错的设计方案。 - kba

这取决于这是一个“只有一两次”的事情，那么我的方法就可以了，还是一个“在我们的代码中会经常发生”的事情，那么我完全同意你的观点，使用组合或访问者模式（或类似模式）会更好。哦，刚看到楼主的澄清，表明后者 - 他应该使用访问者或接口。 - user949300