Анализаторы кода: PMD и FindBugs

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()) тест, и пока у меня нет никаких проблем.

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

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

person hypercube    schedule 10.10.2009    source источник
comment
Ну .. что я могу сказать? Я любопытный человек :-). Спасибо за ссылку.   -  person hypercube    schedule 11.10.2009
comment
Спасибо @Svante за правки.   -  person hypercube    schedule 11.10.2009


Ответы (2)


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

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

Эти файлы находятся в каталоге 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)

Сложность - это количество точек принятия решения в методе плюс одна для записи метода. Точки принятия решения - это «если», «пока», «для» и «метки случая». Как правило, 1–4 означает низкую сложность, 5–7 - среднюю сложность, 8–10 - высокую сложность и 11+ - очень высокую сложность.

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

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

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

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

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

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

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

person Pascal Thivent    schedule 10.10.2009
comment
Спасибо, мужик, как всегда, отличная обратная связь. Хотя цикломатика звучит пугающе, сейчас это не так уж и плохо :-). Я с нетерпением жду ваших следующих правок к этому ответу, и на данный момент он выглядит как главный кандидат на лучший ответ. На самом деле я обнаружил, что и PMD, и плагины FindBugs имеют выбираемые пользовательским интерфейсом проверки предупреждений. Я плохо просил об этом. - person hypercube; 11.10.2009
comment
Что ж ... скомпилируйте и отредактируйте ответ, чтобы я мог выбрать его как лучший ответ :-). Спасибо. - person hypercube; 11.10.2009
comment
Сценарий примет душ и затем поработает над этим. - person Pascal Thivent; 11.10.2009
comment
Похоже, теперь нужно ответить только на 1.4 и 2.1 :-D. - person hypercube; 11.10.2009
comment
О боже, похоже, у тебя есть все ответы :-D. Прежде чем рассматривать вопрос (вопросы), есть еще одна вещь ... У меня есть модульная структура приложения (по крайней мере, я надеюсь, что так :-)), и при выборе модуля общий компонент отображает содержимое модуля, ЕСЛИ его instance не равен нулю, ИЛИ загружает модуль, если его экземпляр равен нулю. Это момент, когда мне действительно нужно сказать своему ядру, что какой-то экземпляр мертв, прежде чем пытаться отобразить. Если я этого не сделаю, у меня появятся неприятные ошибки (поля не отображаются, ошибка - неправильные родители и т. Д.). - person hypercube; 11.10.2009
comment
Я использую это как плохую реализацию метода dispose () для некоторого модуля. Я уверен, что это можно сделать более элегантно, и открыт для предложений. Возможно, мне стоит погуглить, выполнить поиск или ответить на другой вопрос о модульной структуре - лучшие практики и инструкции. - person hypercube; 11.10.2009
comment
Не могли бы вы отредактировать ответ, чтобы я мог поставить там +1? Я хотел сделать это (второй раз), но вместо добавления +1 он добавил -1, и теперь он говорит, что я не могу изменить голосование, потому что оно слишком старое, если ответ не будет отредактирован. - person hypercube; 11.10.2009
comment
Итак, вы добавили в текст 3 символа :-). Хорошо, это по-прежнему лучший ответ. Спасибо! - person hypercube; 11.10.2009
comment
Пожалуйста. Что касается дополнительных допросов, предлагаю задать еще один вопрос. - person Pascal Thivent; 12.10.2009

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

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

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

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

person Nettogrof    schedule 10.10.2009
comment
Я использую что-то вроде Dog instance = null, чтобы заставить некоторые визуальные компоненты перерисовывать при определенных событиях. - person hypercube; 11.10.2009
comment
Ваше определение цикломатической сложности неверно. Цикломатическая сложность метода - это количество независимых путей метода. См. en.wikipedia.org/wiki/Cyclomatic_complexity. - person Pascal Thivent; 11.10.2009
comment
Ах .. снова сценарий :-). Думаю, это урок для меня, чтобы проверить ответы. Проверю и ваши :-P. Спасибо. - person hypercube; 11.10.2009
comment
Мое плохое, ты прав, Паскаль Тивент. Отредактирую свой ответ, чтобы не путать людей. - person Nettogrof; 11.10.2009
comment
Да, это снова сценарий! Я все еще слежу за тобой :) - person Pascal Thivent; 11.10.2009
comment
Хииеее :-). Я рад это слышать :-). - person hypercube; 11.10.2009