这种“instanceof”运算符的使用是否被认为是糟糕的设计？答案

【问题标题】：Is This Use of the "instanceof" Operator Considered Bad Design?这种“instanceof”运算符的使用是否被认为是糟糕的设计？
【发布时间】：2012-01-12 20:08:17
【问题描述】：

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

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

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 类本身中，这对我来说真的没有多大意义，因为理论上我可以对这些记录执行许多不同类型的处理。

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

更新：

将return null 更改为throw new IllegalArgumentException(record);
澄清一下，一个简单的 RecordType.process() 方法不够用的原因有以下三个：首先，处理与 RecordType 相去甚远，不值得在 RecordType 子类中使用自己的方法。此外，理论上可以由不同的处理器执行大量不同类型的处理。最后，RecordType 被设计成一个简单的 DTO 类，其中定义了最少的状态更改方法。

【问题讨论】：

通常，这是一个不好的迹象。但是我们需要了解您要做什么-从问题中并不清楚。一般来说，您的所有类型似乎都具有相同的功能，因此您可以使用接口
RecordType1 和 2 有什么区别？你能让他们遵守同一个界面吗？
你通常不想这样做，但如果你只看到一次表达的逻辑，也许你会接受它。如果你开始在多个地方看到相同的if (a instanceof X) 逻辑，那你就真的不得不给自己一巴掌了。
嗯，RecordType1 和 RecordType2 有共同的子类吗？
仅供参考，Scott Myers 引用的具体引用来自第 39 条“避免继承继承权” - 第一版的第 138 页。他特别指出，C++（大约 1992 年）没有 instanceof 机制是一件好事。

标签： java design-patterns instanceof object-oriented-analysis

【解决方案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。

【讨论】：

这看起来很有希望......那么除了它更复杂之外，使用这种模式的主要好处是什么？难道仅仅是一个人不能通过尝试扩展框架而忘记包含一个案例来破坏框架吗？ trzy 和 cztery 也在哪里（开个玩笑）？
@nomizzz：是的，这种模式强制你在编译时实现子类特定的逻辑。维基百科还带来了另一个我不知道的好处：“访问者对象可以有状态”。我可能应该将您重定向到 GoF 模式书，而不是引用它；-)。是的，代码有点复杂，在某种程度上更不可读。
这是Visitor 的一个非常好的实现。大多数人忘记了泛型类型界限并开始失去强类型。
代替 visitOne(RecordType1 r) & visitTwo(RecordType2 r)，你可以使用重载并做 visit(RecordType1 r) & visit(RecordType2 r)

【解决方案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

【讨论】：

是的，正如我在问题中所讨论的那样，我最初考虑了该解决方案。它有一些问题。首先，该处理与 RecordType 相距甚远，不值得在 RecordType 子类中使用自己的方法。此外，理论上可以由不同的处理器执行大量不同类型的处理。最后，RecordType 被设计成一个简单的 DTO 类，其中定义了最少的状态更改方法。
@ivowiblo 为什么不按照 OP 的建议保留 RecordType 抽象并将 process() 方法抽象化？
因为我不知道它是抽象的。如果示例代码不是 1 或 2，则返回 null，因此我将其留给了基类。
@nomizzz 所以，双分派是最好的。更多考虑到您期待“各种不同类型的处理”。

【解决方案3】：

另一种可能的方法是使 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;
   }
}

注意几个错别字 - 我想我都改正了。

【讨论】：

通常组合/接口优于继承，但仍然是一个不错的公平设计方案。
取决于这是否是“只有一次或两次的事情”，在这种情况下我的方法是可以的，或者“这将在我们的代码中发生很多”的事情，在这种情况下我绝对同意与您一起，组合或访客模式（或类似模式）会更好。 HMM，刚刚看到OP的澄清，表明后者 - 他应该使用Visitor或接口。

【解决方案4】：

设计是达到目的的一种手段，在不了解您的目标或限制条件的情况下，没有人可以判断您的设计在特定情况下是否优秀，或者如何改进。

但是，在面向对象的设计中，将方法实现保留在单独的类中同时仍然为每种类型分别实现的标准方法是 visitor pattern。

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

RecordType processed = process(new RecordType3());

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

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

换句话说，假定无法访问的代码路径应该引发异常，而不是导致未定义的行为。

【讨论】：

【解决方案5】：

在你的例子中，一个想法是糟糕的设计，在适用时不使用 visitor 模式。

另一个是效率。与其他技术相比，instanceof 相当慢，例如使用相等性与 class 对象进行比较。

当使用 visitor 模式时，通常一个有效而优雅的解决方案是使用Map 在支持class 和 Visitor 实例之间进行映射。带有instanceof 检查的大型if ... else 块将非常无效。

【讨论】：

【解决方案6】：

违背了SOLID的开闭原则

【讨论】：