Ошибки, найденные плагином FindBugs Eclipse

Question

используя плагин поиска ошибок, я нашел эти ошибки, но не понимаю, почему это было замечено как ошибка в коде. Кто-нибудь знает и дает мне правильное объяснение по этому поводу? Спасибо.

Исходный код - https://drive.google.com/open?id=1gAyHFcdHBShV-9oC5G7GeOtCGf7bXoso;

Пациент. java: 17 Patient.generatePriority () использует метод RandomDown nextDouble для генерации случайного целого числа ; использование nextInt более эффективно [Of Concern (18), нормальное доверие]

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

ExaminationRoom. java: 25 ExaminationRoom определяет equals и использует Object.hashCode () [Of Concern (16), нормальное доверие ]

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

ExaminationRoom. java: 15 ExaminationRoom определяет CompareTo (ExaminationRoom) и использует Object.equals () [Концерна (16), Нормальная достоверность]

    // 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;
    }

Больница . java: 41 Значение неверного месяца 12 передано новому java .util.GregorianCalendar (int, int, int) в Hospital.initializeHospital () [Страшно (7), Нормальная достоверность]

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

Person. java: 29 Возвращаемое значение String.toLowerCase () игнорируется в Person.getFullName () [Scariest (3), высокая достоверность]

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

}

Answer 1

1. Не создавайте новый объект Random каждый раз.
2. Используйте random.nextInt(n).
3. Определите метод hashCode в ExaminationRoom.
4. Если ваш метод compareTo будет несовместимым с equals(), может быть или не быть в порядке.
5. Используйте LocalDate вместо GregorianCalendar.
6. Возьмите и используйте возвращаемые значения из String.toLowerCase() и Character.toUpperCase().
7. Рассматривайте SpotBugs как более новую альтернативу FindBugs.

Подробнее

Random

Создание нового объекта Random каждый раз, когда вам нужно, дает ваши более бедные псевдослучайные числа с высоким риском повторения чисел. Объявите переменную stati c, содержащую объект 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 не сообщал об этом как об ошибке в последующих анализах, у вас есть два варианта:

1. Вставить аннотацию, указывающую FindBugs игнорировать «ошибку».
2. Создать FindBugs игнорировать файл XML, включая этот пункт.

Извините, я не помню подробностей каждого из них, но ваша поисковая система должна быть полезной.

Дон GregorianCalendar

Класс GregorianCalendar плохо спроектирован и давно устарел. Я предлагаю вам удалить его из своего кода и использовать вместо него LocalDate из java .time, современный Java API даты и времени.

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() возвращают новую строку со всеми буквами в нижнем регистре (в соответствии с правилами локали JVM по умолчанию, сбивает с толку). Вторая строка также не изменяет никаких символов, потому что Java является вызовом по значению (ищите его), поэтому ни один метод не может изменить переменную, переданную в качестве аргумента. Вы даже не передаете переменную, а возвращаете значение из другого метода. Также Character.toUpperCase() возвращает новый символ в нижнем регистре.

Что вам нужно сделать, это получить значения, возвращенные из этих двух вызовов метода, используйте операцию подстроки для удаления первой буквы из строчной версии имени и объединить прописную версию этой буквы с остальной частью строчной буквы. Если это сложно, я уверен, что ваша поисковая система может найти примеры того, где и как это делается.

Немного в стороне: вы можете подумать дважды, прежде чем заставить доктора Джека Макнеила написать Макнейл и доктор Людвиг фон Солсбургский как Фон Саулсбург .

SpotBugs

Это только то, что я слышал, я не проверял себя. Исходный код FindBugs был принят проектом под названием SpotBugs. Говорят, что SpotBugs разрабатывается более активно, чем FindBugs. Так что вы можете рассмотреть вопрос о переключении. Я сам являюсь счастливым пользователем SpotBugs в моей повседневной работе.

Ссылки

• Какие проблемы следует учитывать при переопределении equals и hashCode в Java?
• Документация Comparable, объясняющая, что означает, что compareTo() не соответствует equals() и как go об этом.
• Oracle учебник : Дата и время , объясняющие, как использовать java .time.
• Чика go Надежда , в которой Джек Макнил назван Ортопеди c Хирург.
• Dr. Джек с упоминанием доктора Людвига фон Солсбурга в актерском составе.
• SpotBugs

Answer 2

Первое, что нужно помнить об инструментах «поиска ошибок», это то, что они, как правило, являются лишь рекомендациями. С учетом сказанного:

Класс GregorianCalendar считает месяцы с 0, что означает 0 - январь, 11 - декабрь. 12 представляет 13-й месяц, который не существует. Поскольку функция ожидает int, а вы дали ей int, ошибка компилятора не генерируется, хотя это, безусловно, ошибка. В этой статье хорошо объясняются причины обновления и приводятся примеры использования новых API: https://www.baeldung.com/java-8-date-time-intro

В случае сомнений вы всегда можете проверить документацию. В этом случае класс Calendar (который расширяется GregorianCalendar) деласкирует постоянную c константу public static final int JANUARY = 0; Это подтверждает, что январь действительно равен 0, но также указывает, что мы можем использовать эту константу в нашем коде. Возможно, вы найдете new GregorianCalendar(1988, Calendar.JANUARY, 10) более читабельным.

Возможно, вы также захотите перейти на более современные и стандартные системы, используемые для работы со временем. Библиотеки Java 8 Time являются «новым стандартом», и их обязательно стоит изучить.

Во-вторых, String s являются неизменными в Java. Это означает, что после создания String его значение никогда не может быть изменено. Это может противоречить вашей интуиции, поскольку вы, возможно, видели такой код:

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

Однако это не изменяет строку s. Вместо этого s + " world" создает новый String и присваивает его переменной s.

Аналогично, s.toLowerCase() не меняет то, чем является s, он только генерирует новый String который вы должны назначить.

Вы, вероятно, хотите firstName = firstName.toLowerCase();

В вашем первом примере, ничего сразу не выскакивает мне как "плохое", но если вы посмотрите на сообщения, сгенерированные вашим инструмент, они помечают первый пример как «Of Concern», но помечают другие (например, string.toLowerCase() пример) как «Scary» / «Scariest». Хотя я не знаком с этим инструментом в частности, я предполагаю, что это скорее указывает на «запах кода», а не на фактическую ошибку.

Возможно, посмотрите на Unit Testing, если вы хотите убедиться, что ваш код работает.