FindBugs反对匿名内部类

Question

FindBugs反对匿名内部类

11

这段代码：

Set<Map.Entry<String, SSGSession>> theSet =  new TreeSet<Map.Entry<String, SSGSession>>(new Comparator<Map.Entry<String, SSGSession>>() {

        @Override
        public int compare(final Map.Entry<String, SSGSession> e1, final Map.Entry<String, SSGSession> e2) {
            return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime());
        }
    }));

触发了Sonar中的一个违规行为，触发了Findbugs规则"SIC_INNER_SHOULD_BE_STATIC_ANON"，该规则的描述如下：

这个类是一个内部类，但它没有使用其嵌入式引用来引用创建它的对象。这个引用会使得类的实例变大，并且可能会比必要的时间长保留对创建对象的引用。如果可能的话，应该将该类改为静态内部类。由于匿名内部类无法被标记为静态，因此需要重构内部类使其成为命名内部类。

真的吗？这不是非常挑剔吗？我真的应该重构匿名内部类中的一个一行方法来节省额外引用的成本吗？在这种情况下，它不可能保留对引用比必要更长的引用。

我们的编码标准严格执行"零Sonar违规"，我不介意这样做，但我强烈倾向于在这里争论一个//NOSONAR，因为在我看来，将一个一行方法提取到静态内部类中使得代码稍微难以理解。

Java纯粹主义者怎么想？

- Steve Atkinson

1

由于该类是无状态的，您可以更进一步地将比较器声明为“静态常量”，对于所有调用使用相同的实例，而不是每次创建一个新实例。 - Ian Roberts

1

静态代码分析工具的目的不就是要挑剔吗？对此进行抱怨有明显的技术原因。如果您不想在某个项目中使用它，只需禁用该规则即可。 - hyde

2

为了继续进行，即使在某些情况下似乎不需要应用某些规则，我会采取这样的立场：禁用该规则需要一个强有力的理由，而这并不足够强有力。要么禁用整个规则，要么遵循它。 - hyde

嗯，我通常对大多数findbugs规则都还可以接受，但这个规则却非常苛刻。这个特定的（基于Swing的小型内部测试工具）应用程序可能会使用很多匿名内部类，而我无法控制Sonar启用/禁用哪些规则，因为这是开发中心的标准规定，但由于这是一个内部工具，我可以为其创建一个专用且稍微宽松一点的配置文件。虽然如此，我仍希望在内部项目上也能完全符合规定。 - Steve Atkinson

4个回答

4

为了完整起见，我想添加另一种变体到已经提供的优秀答案中。为Comparator定义一个常量，并使用它：

private static final Comparator<Map.Entry<String, SSGSession>> BY_STARTTIME =
        new Comparator<Map.Entry<String, SSGSession>>() {
    @Override
    public int compare(final Map.Entry<String, SSGSession> e1,
            final Map.Entry<String, SSGSession> e2) {
        return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime());
    }
};

private void foo() {
    Set<Map.Entry<String, SSGSession>> theSet =
        new TreeSet<Map.Entry<String, SSGSession>>(BY_STARTTIME);
}

这个做法可以帮您省去像hyde的回答中的额外类。不过，hyde的做法更好，因为它允许您声明Comparator可序列化（因为它没有状态）。如果Comparator不可序列化，那么您的TreeSet也将不可序列化。

- barfuin

2

这里有三个解决方案，其中最好的是你无法控制的：

扩展Java语法：
```
... theSet = new static Comparator ...
```
声明并使用静态类，如所述。

在这种情况下忽略警告：

@SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON")
... your method ...

我更喜欢第一个，但如果需要很长时间才能实现，那么在忽略整个项目的规则之前，我会选择最后一个。选择规则应该需要一些代价来覆盖它；否则它只是一个惯例或建议。

- David Harkness

0

请注意：匿名内部类是泄漏内存的好方法，特别是在JEE bean中使用时。像这样简单的东西：

new HashMap<>() {{
    put"("a","b");
}};

在使用@javax.ejb.Singleton注释的bean中可能会导致多个实例保持活动状态，因为该Map保留对bean的引用。

- user158037

网页内容由stack overflow 提供, 点击上面的

可以查看英文原文，
原文链接

- hyde · Accepted Answer

将注释转换为答案，首先我可以被说服将其作为匿名内部类，即使存在明显的技术原因需要挑剔。

不过，我要说：遵循你所制定的规则。规则创造了一致性，当所有的代码都以同样的方式编写时，整个代码库更容易理解。如果某个规则不好，请在所有地方禁用它。

当有例外情况时，也需要解释为什么会出现这种例外情况：对于阅读代码的人来说，这是额外的思考负担，在代码审查中讨论的额外事项等。只有在你能够认为这是某种特殊情况的情况下才在个别情况下禁用规则。

另外，我不确定将其作为静态类是否更容易理解，即使它增加了一些样板代码（如果下面的代码不是100％正确的Java，我很抱歉，我的Java有点生疏，欢迎提供建议）：

Set<Map.Entry<String, SSGSession>> theSet 
    = new TreeSet<Map.Entry<String, SSGSession>>(new SSGSessionStartTimeComparator());

然后在文件的其他位置，与其他静态类一起：

static class SSGSessionStartTimeComparator extends Comparator<Map.Entry<String, SSGSession>>() {
    @Override
    public int compare(final Map.Entry<String, SSGSession> e1, final Map.Entry<String, SSGSession> e2) {
        return e2.getValue().getStartTime().compareTo(e1.getValue().getStartTime());
    }
}