3

如果无法修改 Event 接口，如何重构以下方法？PMD 报告太复杂，findbugs 报告 ITC_INHERITANCE_TYPE_CHECKING。还有幻数，如 3、4、5 等。

 public int getEventCode(Event event) {
        if (event instanceof OneEvent) {
            return 1;
    }
        if (event instanceof TwoEvent) {
            return 2;
        }
        if (event instanceof ThreeEvent) {
            return 3;
        }
        if (event instanceof FourEvent) {
            return 4;
        }
        if (event instanceof FiveEvent) {
            return 5;
        }
        if (event instanceof SixEvent) {
            return 6;
        }
        if (event instanceof SevenEvent) {
            return 7;
        }
        if (event instanceof EightEvent) {
            return 8;
        }
        if (event instanceof NineEvent) {
            return 9;
        }
        if (event instanceof TenEvent) {
            return 10;
        }
        return event.getClass().hashCode() + 10;
    }
4

4 回答 4

3

您可以使用List<Class<?>>例如：

private static final List<Class<? extends Event>> EVENT_CLASSES
    = Arrays.asList(OneEvent.class, ...);

然后：

public int getEventCode(final Event event)
{
    final Class<? extends Event> c = event.getClass();
    final int index = EVENT_CLASSES.indexOf(c);
    return index != -1 ? index + 1 : c.hashCode() + 10;
}

注意：要求事件是确切的类，而不是派生的（即，OneEvent不是OneDerivedEvent）。否则，测试会稍微复杂一些，但仍然可行。

至于：

findbugs 报告 ITC_INHERITANCE_TYPE_CHECKING

是的，这是因为instanceof支票。

但是：一开始的代码存在一个根本缺陷。不能保证.hashCode()在两个不同的 JVM 执行之间返回相同的值。更重要的是，它允许返回负值。这意味着它可以返回例如 -4 作为值；这意味着 6 将返回“其他事件”，因此与SixEvent.

考虑重构！

于 2013-07-05T09:11:32.947 回答
1 
public int getEventCode(OneEvent event) {
    return 1;
}
public int getEventCode(TwoEvent event) {
    return 2;
}
// etc.

这不好 - 但如果您不能更改 Event 类，这可能是满足您要求的最面向对象的方式。这是用多态性代替条件，而不改变有问题的类

于 2013-07-05T15:27:58.483 回答
0

代码库中是否存在一些此类类型的块？如果是这样，用多态性替换条件http://refactoring.com/catalog/replaceConditionalWithPolymorphism.html可能会有所帮助。

于 2013-07-05T09:11:35.190 回答
0

好吧，使用 instanceof 并不好，但如果您无法更改事件接口，甚至添加“int getType()”或类似内容，至少您可以重构您的代码：

使用“else if”结构而不是 if 结构。我建议将继承更深的事件类型放在第一位，最后考虑更通用的事件声明（如果您测试事件 instanceof Event，您将始终为真）。

这样至少可以降低复杂性。

假设您的所有事件都直接实现事件接口并且没有其他继承问题需要检查：

public int getEventCode(Event event) {
    int result = event.getClass().hashCode() + 10;
    if (event instanceof OneEvent) {
        result = 1;
    }
    else if (event instanceof TwoEvent) {
        result = 2;
    }
    else if (event instanceof ThreeEvent) {
        result = 3;
    }
    else if (event instanceof FourEvent) {
        result = 4;
    }
    else if (event instanceof FiveEvent) {
        result = 5;
    }
    else if (event instanceof SixEvent) {
        result = 6;
    }
    else if (event instanceof SevenEvent) {
        result = 7;
    }
    else if (event instanceof EightEvent) {
        result = 8;
    }
    else if (event instanceof NineEvent) {
        result = 9;
    }
    else if (event instanceof TenEvent) {
        result = 10;
    }
    return result;
}

我还更改了您的方法，因此它只会声明一个返回变量，这通常被认为是一种更好的编码实践，因为它也有助于降低复杂性，即使在这种特殊情况下它没有真正的技术优势。

但正如我所说，如果 TenEvent 扩展 FiveEvent 您也可能会遇到问题。在这种情况下，我建议使用您收到的事件实例的类，如下所示：

public int getEventCode(Event event) {
    int result = event.getClass().hashCode() + 10;
    String eventClass = event.getClass().getSimpleName();
    if ("OneEvent".equals(eventClass) {
        result = 1;
    }
    else if ("TwoEvent".equals(eventClass)) {
        result = 2;
    }
    return result;
}

我变得懒惰，只写示例中的前两个，所以你明白了......

甚至更好：

public int getEventCode(Event event) {
    int result = event.getClass().hashCode() + 10;
    Class eventClass = event.getClass();
    if (OneEvent.class.equals(eventClass) {
        result = 1;
    }
    else if (TwoEvent.class.equals(eventClass)) {
        result = 2;
    }
    return result;
}
于 2013-07-05T09:16:13.013 回答