FindBugs 插件 Eclipse 发现的错误答案

【问题标题】：Bugs found by FindBugs plugin EclipseFindBugs 插件 Eclipse 发现的错误
【发布时间】：2020-05-11 22:13:37
【问题描述】：

使用错误查找器插件，我发现了这个错误，但不明白为什么它被视为代码中的错误。有人知道这些并给我适当的解释吗？谢谢。

源代码 - https://drive.google.com/open?id=1gAyHFcdHBShV-9oC5G7GeOtCGf7bXoso;

Patient.java:17 Patient.generatePriority() 使用 Random 的 nextDouble 方法生成随机整数；使用 nextInt 更有效 [Of Concern(18), Normal confidence]

 public int generatePriority(){
    Random random = new Random();
    int n = 5;
    return (int)(random.nextDouble()*n);
 }

ExaminationRoom.java:25 ExamRoom 定义 equals 并使用 Object.hashCode() [Of Concern(16), Normal confidence]

public boolean equals(ExaminationRoom room){
        if (this.getWaitingPatients().size() == room.getWaitingPatients().size()){
            return true;
        }
        else {
            return false;
        }
    }

ExaminationRoom.java:15 ExamRoom 定义 compareTo(ExaminationRoom) 并使用 Object.equals() [Of Concern(16), Normal confidence]

    // Compares sizes of waiting lists
    @Override
    public int compareTo(ExaminationRoom o) {
        if (this.getWaitingPatients().size() > o.getWaitingPatients().size()){
            return 1;
        }
        else if (this.getWaitingPatients().size() < o.getWaitingPatients().size()){
            return -1;
        }
        return 0;
    }

Hospital.java:41 错误的月份值 12 传递给 Hospital.initializeHospital() 中的新 java.util.GregorianCalendar(int, int, int) [可怕(7)，正常置信度]

    doctors.add(new Doctor("Hermione", "Granger", new GregorianCalendar(1988, 12, 10), Specialty.PSY, room102));

Person.java:29 在 Person.getFullName() 中忽略 String.toLowerCase() 的返回值 [Scariest(3)，高置信度]

public String getFullName(){
    firstName.toLowerCase();
    Character.toUpperCase(firstName.charAt(0));
    lastName.toLowerCase();
    Character.toUpperCase(lastName.charAt(0));
    return firstName + " " + lastName;

}

【问题讨论】：

月份从 0 开始。所以 12 月是 11 日。你已经过了 12 岁。
我建议你不要使用GregorianCalendar。该课程设计不良且早已过时。关于它的一个令人困惑的事情可能是它最突出的设计错误是它从 0 到 11 个月。而是使用现代 java.time 中的LocalDate。 LocalDate.of(1988, Month.DECEMBER, 10) 或 LocalDate.of(1988, 12, 10)。没有混乱。
顺便说一下，您的代码可以简化以提高可读性：public boolean equals(ExaminationRoom other){ return getWaitingPatients().size() == other.getWaitingPatients().size(); } 和 @Override public int compareTo(ExaminationRoom other) { return getWaitingPatients().size() - other.getWaitingPatients().size(); }。

标签： java eclipse findbugs spotbugs

【解决方案1】：

关于“错误查找器”工具，首先要记住的是，它们通常只是指南。话虽如此：

GregorianCalendar 类从 0 开始计算月份，这意味着 0 是一月，11 是十二月。 12 代表不存在的第 13 个月。由于该函数需要一个int，而您给了它一个int，因此不会产生编译器错误，即使这肯定是一个错误。这篇文章很好地解释了升级的原因，并举例说明了如何使用新的 API：https://www.baeldung.com/java-8-date-time-intro

如有疑问，您可以随时查看文档。在这种情况下，Calendar 类（GregorianCalendar 扩展）删除了一个静态常量public static final int JANUARY = 0; 这证实了 january 确实是0，但也表明我们可以在我们的代码中使用这个常量。您可能会发现 new GregorianCalendar(1988, Calendar.JANUARY, 10) 更具可读性。

您可能还想考虑切换到用于处理时间的更现代和标准的系统。 Java 8 Time 库是“新标准”，绝对值得研究。

其次，Strings 在 Java 中是不可变的。这意味着一旦创建了String，它的值就永远不能改变。这可能与您的直觉相反，因为您可能已经看过如下代码：

String s = "hello";
s = s + " world";

但是，这不会修改字符串s。相反，s + " world" 会创建一个新的String，并将其分配给变量s。

同样，s.toLowerCase() 不会改变 s 是什么，它只会生成一个您必须分配的新 String。

你可能想要firstName = firstName.toLowerCase();

对于您的第一个示例，没有任何内容立即让我觉得“不好”，但是如果您查看工具生成的消息，他们会将第一个示例标记为“关注”，但将其他示例标记为（例如 @ 987654341@ 示例）作为“可怕”/“最可怕”。虽然我对这个工具并不特别熟悉，但我想这更像是一种“代码味道”，而不是真正的错误。

如果您想让自己确信您的代码有效，也许可以研究一下单元测试。

【讨论】：

谢谢。这让我对代码有所启发。第一个怎么样？
同意，我的措辞听起来太笼统了。我会编辑我的答案。我想当有疑问时，只需查看文档

【解决方案2】：

不要每次都创建新的Random 对象。
使用random.nextInt(n)。
在ExaminationRoom 中定义hashCode 方法。
让您的compareTo 方法与equals() 不一致可能会也可能不会。
使用LocalDate 而不是GregorianCalendar。
提取并使用来自String.toLowerCase() 和Character.toUpperCase() 的返回值。
将 SpotBugs 视为 FindBugs 的更新替代品。

详情

Random

每次您需要一个新的Random 对象时，都会产生较差的伪随机数，并且数字重复的风险很高。在您的方法之外声明一个包含Random 对象的静态变量，并在声明中对其进行初始化（Random 是线程安全的，因此您可以安全地这样做）。要绘制一个从 0 到 4 的伪随机数，请使用

int n = 5;
return random.nextInt(n);

它不仅更高效（正如 FindBugs 所说），我首先发现它更具可读性。

hashCode()

@Override
public int hashCode() {
    return Objects.hash(getWaitingPatients());
}

compareTo()

您向我们展示的equals 方法似乎与这里的 FindBugs 相矛盾。不过，它看起来确实有点滑稽。如果两个候诊室有相同数量的候诊病人，它们是否被视为相同？请再想一想。如果您最终决定它们不相等但应该毫无区别地归类到同一位置，那么您的compareTo 方法与equals() 不一致。如果是这样，请插入说明这一事实的评论。如果您不希望 FindBugs 在后续分析中将此作为错误报告，您有两种选择：

插入一个注释，告诉 FindBugs 忽略“错误”。
创建一个包含此点的 FindBugs 忽略 XML 文件。

很抱歉，我不记得每个细节，但您的搜索引擎应该会有所帮助。

不要使用GregorianCalendar

GregorianCalendar 类设计不佳且早已过时。我建议你从你的代码中删除它，并使用现代 Java 日期和时间 API java.time 中的LocalDate。

doctors.add(new Doctor("Hermione", "Granger", LocalDate.of(1988, Month.DECEMBER, 10), Specialty.PSY, room102));

String.toLowerCase()

这已经在另一个答案中得到处理。将名称更改为首字母大写，其余字母小写并不像听起来那么简单。

firstName.toLowerCase();
Character.toUpperCase(firstName.charAt(0));

这两行中的第一行不会修改字符串firstName，因为字符串被设计为不可变的，toLowerCase() 返回一个所有字母都小写的new字符串（根据JVM的默认语言环境的规则，令人困惑）。第二行也没有修改任何字符，因为 Java 是按值调用（查找），所以没有方法可以修改作为参数传递的变量。你甚至没有传递一个变量，而是来自不同方法的返回值。 Character.toUpperCase() 还返回一个 new 小写字符。

您需要做的是获取从这两个方法调用返回的值，使用子字符串操作从名称的小写版本中删除第一个字母，并将该字母的大写版本与小写字符串的其余部分。如果它很复杂，我相信您的搜索引擎可以找到在哪里以及如何完成的示例。

顺便说一句：在强制 Jack McNeil 博士写成 Mcneil 和 Ludwig von Saulsbourg 博士写成 Von saulsbourg 之前，您可能需要三思而后行。

SpotBugs

这只是我听到的，我没有检查过自己。 FindBugs 的源代码已被一个名为 SpotBugs 的项目接管。他们说 SpotBugs 的开发比 FindBugs 更积极。所以你可以考虑换。在我的日常工作中，我自己是一个快乐的 SpotBugs 用户。

链接

What issues should be considered when overriding equals and hashCode in Java?
Documentation of Comparable 解释 compareTo() 与 equals() 不一致的含义以及如何处理。
Oracle tutorial: Date Time 解释如何使用 java.time。
Chicago Hope 将 Jack McNeil 列为骨科医生。
Dr. Jack 在演员阵容中提到 Ludwig von Saulsbourg 博士。
SpotBugs

【讨论】：