PMD - предупреждения анализатора кода - PullRequest
15 голосов
/ 05 января 2011

Я использую PMD для анализа кода, и он выдает несколько высокоприоритетных предупреждений, которые я не знаю, как исправить.

1) Avoid if(x!=y)..; else...; Но что мне делать, если мне нужна эта логика?То есть мне нужно проверить, если x!=y?Как я могу сделать рефакторинг?

2) Use explicit scoping instead of the default package private level. Но этот класс действительно используется только внутри пакета.Какой модификатор доступа я должен использовать?

3) Parameter is not assigned and could be declared final. Должен ли я добавить окончательное ключевое слово во все места, на которые PMD указал это предупреждение?

Ответы [ 5 ]

30 голосов
/ 05 января 2011

Избегайте отрицания: Вместо if( x!=y ) doThis() else doThat(), сначала проверьте положительный случай, потому что люди / люди склонны любить положительные вещи больше, чем отрицательные. Это искажает мозг, заставляя вспомнить логику при чтении исходного кода. Поэтому вместо этого напишите:

 if ( x!=y ) doThis() else doThat()       // Bad - negation first
 if ( x==y ) doThat() else doThis()       // Good - positive first

Явные обзорное: По сайт PMD , это спорное правило. Вы можете ненавидеть это, кому-то еще это нравится. Что вы должны сделать, это сделать все поля в ваших классах закрытыми. Кажется, что есть поле или метод (не класс) с видимостью пакета, например как то так:

 class Foo {
   /* private missing */ Object bar;
 }

Окончательные параметры: Параметры метода должны быть окончательными, чтобы избежать случайного переназначения. Это просто хорошая практика. Если вы используете Eclipse, помощник по контенту даже предоставляет быстрое исправление под названием «Изменить модификаторы на финальный, где это возможно» . Просто выделите весь код в редакторе с помощью Ctrl-a, а затем нажмите Ctrl-1.

6 голосов
/ 05 января 2011

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

1 - преобразовать его в логику if (x == y) ... else .... Просто избегайте негативных условий, если из-за статистики они затрудняют понимание кода

2 - я бы не включил это правило.

3 - Многие люди объявляют множество полей и переменных окончательными. Особенно, когда они хотят убедиться или выразить, что значение переменной не должно быть изменено в методе. Если вам это не нравится, отключите это правило.

4 голосов
/ 05 января 2011

Относительно первого пункта (неравенства) есть две проблемы:

1) Удобочитаемость двойного отрицания.

Скажем, у вас есть:

if(x!=y) { false clause } else { true clause }

Второе предложение выполняется, если «не х не равно y».

Это можно переписать как:

if (x==y) {true clause } else {false clause}.

2) Правильность: если x и y не являются примитивами, использование if(!x.equals(y)) более безопасно.Это эквивалентно использованию == вместо .equals () и может привести к очень серьезным ошибкам.

4 голосов
/ 05 января 2011

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

1) Он хочет, чтобы вы перевернули логику

if(x==y) {
    //old else clause
} else {
    //old if clause
}

2) Если для пакета действительно необходим правильный доступ, модификатор доступа для добавления отсутствует. Я недостаточно знаком, чтобы знать, есть ли способ подавить это конкретное предупреждение.

3) Проблема стиля. Некоторые люди хотят окончательного на все, что может быть. Другие думают, что это добавляет слишком много беспорядка для небольшого количества информации. Если вы находитесь в последнем лагере, выключите это предупреждение.

1 голос
/ 03 июня 2012

Вы также можете использовать // NOPMD в конце любой строки, где вы не хотите проверять правила PMD.

Например, для приведенного выше кода вы можете подавить проверку PMD, задав1004 *

class Foo {
   /* private missing */ Object bar; // NOPMD
 }

Обратите внимание, что приведенный выше комментарий может скрытно подавить другие предупреждения в строке той же .

...