3

好的，在使用PMD和FindBugs代码分析器审查了一些代码之后，我能够对审查过的代码进行很大的更改。但是，有些事情我不知道如何解决。我将在下面重复它们，并且（为了更好地参考）我会给每个问题一个数字。随意回答任何/所有问题。谢谢你的耐心。

1. 即使我已经删除了一些规则，但重新评估代码后相关的警告仍然存在。知道为什么吗？

2.请看声明：

    private Combo comboAdress;

    private ProgressBar pBar;

以及 getter 和 setter 对对象的引用：

    private final Combo getComboAdress() {
  return this.comboAdress;
 }

 private final void setComboAdress(final Combo comboAdress) {
  this.comboAdress = comboAdress;
 }

 private final ProgressBar getpBar() {
  return this.pBar;
 }

 private final void setpBar(final ProgressBar pBar) {
   this.pBar = pBar;
 }

现在，我想知道为什么第一个声明没有给我任何关于 PMD 的警告，而第二个声明却给了我以下警告：

Found non-transient, non-static member. Please mark as transient or provide accessors.

有关该警告的更多详细信息，请点击此处。

3. 这是另一个警告，也是由 PMD 给出的：

    A method should have only one exit point, and that should be the last statement in the method

有关该警告的更多详细信息，请点击此处。

现在，我同意这一点，但是如果我写这样的东西怎么办：

public void actionPerformedOnModifyComboLocations() {
    if (getMainTree().isFocusControl()) {
        return;
    }
    ....//do stuffs, based on the initial test
}

我倾向于同意这条规则，但如果代码的性能表明存在多个退出点，我该怎么办？

4. PMD 给了我这个：

Found 'DD'-anomaly for variable 'start_page' (lines '319'-'322').

当我声明类似：

String start_page = null;

如果我删除了对 null 的赋值，我会删除这个信息（警告级别是信息），但是..我从 IDE 收到一个错误，说变量可能在稍后的代码中未初始化。所以，我有点坚持。压制警告是你能做的最好的事情吗？

5. PMD警告：

Assigning an Object to null is a code smell.  Consider refactoring.

这是 GUI 组件的单一使用的情况或返回复杂对象的方法的情况。在 catch() 部分中将结果分配给 null 是为了避免返回不完整/不一致的对象。是的，应该使用 NullObject，但在某些情况下我不想这样做。那我应该取消那个警告吗？

6. FindBugs 警告 #1：

Write to static field MyClass.instance from instance method MyClass.handleEvent(Event)

在方法中

@Override
public void handleEvent(Event e) {
    switch (e.type) {
        case SWT.Dispose: {
            if (e.widget == getComposite()) {
                MyClass.instance = null;
            }
       break;
         }
    }
}

静态变量

private static MyClass instance = null;

该变量允许我测试表单是否已经创建并且是否可见，并且在某些情况下我需要强制重新创建表单。我在这里看不到其他选择。有什么见解吗？（MyClass 实现了 Listener，因此重写了 handleEvent() 方法）。

7. FindBugs 警告 #2：

Class MyClass2 has a circular dependency with other classes

此警告基于其他类的简单导入显示。我是否需要重构这些导入以使此警告消失？还是问题出在 MyClass2 中？

好的，现在说得够多了……期待更新，基于更多的发现和/或你的答案。谢谢。

4

3 回答 3

2

以下是我对您的一些问题的回答：

问题2：

我认为你没有正确地利用这些属性。这些方法应该称为getPBar 和setPBar。

String pBar;

void setPBar(String str) {...}
String getPBar() { return pBar};

JavaBeans 规范指出：

对于可读属性，将有一个 getter 方法来读取属性值。对于可写属性，将有一个 setter 方法来允许更新属性值。[...] 通过 getFoo 和 setFoo 访问器方法为遵循标准 Java 约定的属性构造一个 PropertyDescriptor。因此，如果参数名称是“fred”，它将假定读取器方法是“getFred”，写入器方法是“setFred”。请注意，属性名称应以小写字符开头，在方法名称中将大写。

问题3：

我同意您正在使用的软件的建议。为了可读性，只有一个出口点更好。为了提高效率，使用'return;' 可能会更好。我的猜测是编译器足够聪明，可以始终选择有效的替代方案，我敢打赌，两种情况下的字节码都是相同的。

更多经验信息

我做了一些测试，发现我正在使用的 java 编译器（Mac OS X 10.4 上的 javac 1.5.0_19）没有应用我预期的优化。

我使用以下类进行测试：

public abstract class Test{

  public int singleReturn(){   
     int ret = 0;
     if (cond1())
         ret = 1;
     else if (cond2())
         ret = 2;
     else if (cond3())
         ret = 3;

    return ret;
  }

  public int multReturn(){
    if (cond1()) return 1;
    else if (cond2()) return 2;
    else if (cond3()) return 3;
    else return 0;
  }

  protected abstract boolean cond1();
  protected abstract boolean cond2();
  protected abstract boolean cond3();
}

然后，我分析了字节码，发现对于 multReturn() 有几个 'ireturn' 语句，而对于 singleReturn() 只有一个。此外，singleReturn() 的字节码中还包含了几个goto到 return 语句。

我用非常简单的 cond1、cond2 和 cond3 实现测试了这两种方法。我确保这三个条件同样可以证明。我发现 3% 到 6% 的时间差异一致，有利于 multReturn()。在这种情况下，由于操作非常简单，因此多次返回的影响是相当明显的。

然后我使用更复杂的 cond1、cond2 和 cond3 实现测试了这两种方法，以使不同返回的影响不那么明显。我被结果震惊了！现在 multReturn() 始终比 singleReturn()慢（在 2% 和 3% 之间）。我不知道是什么导致了这种差异，因为其余的代码应该是相等的。

我认为这些意想不到的结果是由 JVM 的 JIT 编译器造成的。

无论如何，我坚持我最初的直觉：编译器（或 JIT）可以优化这类事情，这让开发人员可以专注于编写易于阅读和维护的代码。

问题6：

您可以从您的实例方法调用类方法，并让该静态方法更改类变量。

然后，您的代码类似于以下内容：

public static void clearInstance() {
    instance = null;
}

@Override
public void handleEvent(Event e) {
    switch (e.type) {
        case SWT.Dispose: {
            if (e.widget == getComposite()) {
                MyClass.clearInstance();
            }
       break;
         }
    }
}

这将导致您在 5 中描述的警告，但必须做出一些妥协，在这种情况下，它只是一种气味，而不是错误。

问题7：

这只是一个可能的问题的气味。它不一定是坏的或错误的，你不能仅仅通过使用这个工具来确定。

如果你遇到了真正的问题，比如构造函数之间的依赖关系，测试应该会显示出来。

一个不同但相关的问题是 jar 之间的循环依赖：虽然可以编译具有循环依赖的类，但由于类加载器的工作方式，jar 之间的循环依赖无法在 JVM 中处理。

于 2009-10-17T16:32:20.207 回答
2

  1. 我不知道。无论你做了什么，似乎都不是你想要做的！

  2. 也许声明出现在一个Serializable类中，但类型（例如ComboProgress，它们本身不是可序列化的）。如果这是 UI 代码，那似乎很有可能。我只会评论该类以表明它不应该被序列化。

  3. 这是一个有效的警告。你可以重构你的代码： 

    public void actionPerformedOnModifyComboLocations() {
    if (!getMainTree().isFocusControl()) {
        ....//do stuffs, based on the initial test
    }
}

  4. 这就是我无法忍受静态分析工具的原因。一项null作业显然会让您NullPointerException稍后再接受 s。但是，有很多地方这是无法避免的（例如，使用try catch finallya 来进行资源清理Closeable）

  5. 这似乎也是一个有效的警告，您对static访问权限的使用可能会被大多数开发人员视为代码异味。考虑通过使用依赖注入进行重构，将资源跟踪器注入到您目前使用静态的类中。

  6. 如果您的课程有未使用的导入，则应将其删除。这可能会使警告消失。另一方面，如果需要导入，您可能会有真正的循环依赖，如下所示：

    class A {
    private B b;
}
class B {
    private A a;
}

这通常是一种令人困惑的状态，并且会让您面临初始化问题。例如，您可能不小心在初始化时添加了一些代码A，要求初始化其B实例。如果您将类似B的代码添加A到B.

再次说明为什么我真的不喜欢静态分析工具 - 它们通常只会为您提供一堆误报。循环相关的代码可能工作得非常好，并且记录得非常好。

于 2009-10-17T16:08:57.523 回答
1

对于第 3 点，现在可能大多数开发人员会说单返回规则完全是错误的，并且平均会导致代码更差。其他人则认为它是一个写下来的规则，带有历史凭证，一些破坏它的代码很难阅读，因此不遵守它是完全错误的。

您似乎同意第一个阵营，但缺乏信心告诉工具关闭该规则。

要记住的是，在任何检查工具中编写代码都是一条简单的规则，有些人确实想要它。所以它几乎总是由他们实施。

而很少（如果有的话）执行更主观的“守卫”；身体; 返回计算；' 模式通常产生最容易阅读和最简单的代码。

因此，如果您正在考虑生成好的代码，而不是简单地避免最差的代码，那是您可能确实想要关闭的一条规则。

于 2009-10-17T17:23:03.013 回答