Анализаторы кода: PMD & FindBugs - PullRequest
7 голосов
/ 11 октября 2009

1. Относительно PMD:

1.1 Как настроить проверки PMD, чтобы игнорировать некоторые из них, такие как «Имя переменной слишком короткое или слишком длинное», «Удалить пустой конструктор и т. Д.» - и если я это сделаю, появится другое предупреждение, в котором говорится класс должен иметь несколько статических методов. По сути, класс был пуст для дальнейшего развития, и я хотел бы пока оставить его таким.

1.2. Обязательно ли следовать этому предупреждению?

  A class which only has private constructors should be final

1.3. Что это должно означать?

 The class 'Dog' has a Cyclomatic Complexity of 3 (Highest = 17)

1.4 Как насчет этого? Я бы с радостью изменил это, но в данный момент мне ничего не приходит в голову относительно изменений:

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

2. Относительно FindBugs:

2.1 Неужели так плохо писать в статическое поле, в какой-то момент позже, чем его объявление? Следующий код дает мне предупреждение:

Main.appCalendar = Calendar.getInstance();
Main.appCalendar.setTimeInMillis(System.currentTimeMillis());

где appCalendar - статическая переменная.

2.2 Этот код:

strLine = objBRdr.readLine().trim();

выдает предупреждение:

Immediate dereference of the result of readLine()

, где objBRdr - это BufferedReader(FileReader). Что может случиться? readLine() может быть нулем? Код вложен в while (objBRdr.ready()) test, и пока у меня проблем там ноль.

Обновление 1: 2.2 было исправлено, когда я заменил код на:

strLine = objBRdr.readLine();
    if (strLine != null) {
        strLine = strLine.trim();
    }

Ответы [ 2 ]

9 голосов
/ 11 октября 2009

1.1 Как настроить проверки PMD [...]

PMD хранит конфигурацию правил в специальном хранилище, которое называется XML-файлом Ruleset. Этот файл конфигурации содержит информацию об установленных на данный момент правилах и их атрибутах.

Эти файлы находятся в каталоге rulesets дистрибутива PMD. При использовании PMD с Eclipse, проверьте Настройка PMD .

1.2 Нужно ли следовать этому предупреждению?

A class which only has private constructors should be final

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

Так что на самом деле невозможно извлечь подкласс из класса, каждый конструктор которого является закрытым. Таким образом, маркировка такого класса как final является хорошей идеей (но не обязательной), поскольку она явно предотвращает создание подклассов.

1.3 Что это должно означать?

The class 'Dog' has a Cyclomatic Complexity of 3 (Highest = 17)

Сложность - это количество точек принятия решения в методе плюс одна для записи метода. Решающими моментами являются 'if', 'while', 'for' и 'case case'. Как правило, 1-4 означает низкую сложность, 5-7 означает умеренную сложность, 8-10 означает высокую сложность и 11+ означает очень высокую сложность.

Сказав это, я просто процитирую некоторые части Совокупная цикломатическая сложность не имеет смысла :

[...] Эта метрика имеет значение только в контексте одного метода. Упоминание о том, что класс имеет цикломатическую сложность X, по существу бесполезно.

Потому что цикломатические меры сложности путь в методе, каждый метод имеет по крайней мере, цикломатическая сложность 1, право? Итак, следующий метод получения имеет значение CCN 1:

public Account getAccount(){
   return this.account;
}

Это ясно из этого метода буги что account является собственностью этого учебный класс. Теперь представьте, что этот класс имеет 15 свойств и следует типичной парадигме получения / установки для каждого свойства , и это единственные доступные методы . Это означает, что класс имеет 30 простых методов, каждый из которых имеет значение цикломатической сложности 1. Совокупное значение класса равно 30.

Имеет ли это значение какое-то значение, чувак? Конечно, смотреть со временем может дать что-то интересное; тем не мение, сам по себе, как совокупная стоимость, он по сути бессмысленно. 30 для класс ничего не значит, 30 для метода хоть что-то значит.

В следующий раз, когда вы окажетесь чтение copasetic агрегат Цикломатическое значение сложности для класс, убедитесь, что вы понимаете, как много методов, содержащихся в классе. Если совокупная цикломатическая сложность значение класса 200 - не должно поднять любые красные флаги, пока вы не знаете, количество методов. Более того, если вы обнаружите, что количество методов еще мало значение цикломатической сложности высокий, вы почти всегда найдете сложность локализована для метода . Отлично!

Так что для меня это правило PMD следует соблюдать осторожно (и на самом деле оно не очень ценно).

1.4 Как насчет этого? Я бы с радостью изменил это, но в данный момент мне ничего не приходит в голову относительно изменений:

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

Не уверен, что вы не получите об этом.

2.1 Неужели так плохо писать в статическое поле, в какой-то момент позже, чем его объявление? [...]

МойПредполагается, что вы получите предупреждение, потому что метод содержит несинхронизированную ленивую инициализацию энергонезависимого статического поля. И поскольку компилятор или процессор могут переупорядочивать инструкции, потокам не гарантируется увидеть полностью инициализированный объект, если метод может быть вызван несколькими потоками. Вы можете сделать поле нестабильным, чтобы исправить проблему.

2.2 [...] Immediate dereference of the result of readLine()

Если больше нет строк текста для чтения, readLine() вернет ноль и разыменование, которое сгенерирует исключение нулевого указателя. Так что вам действительно нужно проверить, равен ли результат нулю.

2 голосов
/ 11 октября 2009

Вот какая-то идея / ответ

1.4. Какова причина присвоения нулю объекта? Если вы повторно используете одну и ту же переменную, нет оснований устанавливать ее в null.

2.1. Причина этого предупреждения заключается в том, что все ваши экземпляры класса Main имеют одинаковые статические поля. В вашем основном классе вы могли бы иметь статический календарь appCalendar = Calendar.getInstance ();

о вашем 2.2 вы правы, с проверкой на ноль, вы уверены, что у вас не будет никакого NullPointerException. Мы никогда не знаем, когда ваш BufferedReader может заблокировать / удалить мусор, это случается не часто (по моему опыту), но мы никогда не знаем, когда происходит сбой жесткого диска.

...