Впечатления от Eclipse-плагина PMD

На днях был один пост о полезных плагинах для Eclipse от пользователя gAmUssA, в котором был упомянут в том числе и PMD. Мне этот плагин показался интересным и я решил по-пробовать натравить эту зверушку на свой проект…

Для начала о проекте — это довольно не маленький java проект в 2000+ файлов кода. Так что изучить этому плагину было что. Писался и пишется он на протяжении уже около 4х лет и не менее чем 10 разными программистами, как нашими, так и забугорными.
В специфику проекта вдаваться не буду — особого значения это сейчас не имеет.
Ну в общем понеслась… Поставился он без проблем и лишних вопросов. Запустил я его и минут 5-7 он занимался глубоким анализом всего того бреда, который мы с коллегами годами писали. В итоге в Volations Overview View был выдан результат: такая себе табличка с пакеджами и классами в которых были найдены «ошибки». В тоже время в Volations Outline View можно было видеть описание этих «ошибок» и уже оттуда можно было перейти к конкретной строке кода.
В плагине предусмотрено 5 приоритетов «ошибок»:

1. Error High
2. Error
3. Warning High
4. Warning
5. Information

С 3-го по 5-й слишком незначительны, что бы их упоминать и в большей степени относятся к спорного рода рекомендациям, вроде «A method argument that is never assigned can be declared final» (ну конечно он может быть объявлен финальным, но особого смысла в этом нет, как мне кажется), а также неиспользованным импортам.
Поэтому я сразу выключаю отображение «ошибок» этих приоритетов (плагин легко позволяет это сделать нажатием соответствующей кнопочки в Volations Overview).
Итак, остаются только самые страшные, с точки зрения PMD ошибки.
Среди них я выделил наиболее распространение в моем коде. Учитывая объем этого кода, я считаю результирующие данные достаточно репрезентативной выборкой, чтобы говорить о близких закономерностях в других проектах. :)

Наиболее часто встречающимися ошибками оказались следующие:

1. An empty method in an abstract class should be abstract instead
   Мне кажется, спорное утверждение… Ну вот не нужно мне, что бы наследники в обязательном порядке переопределяли этот метод. Зачем я буду его делать абстрактным?
2. System.out.print is used
   Тут конечно не поспоришь… Кто-то что-то отлаживал и забыл убрать. Стыдно. И это при наличии живого логгера.
3. Overridable method called during object construction
   Это предупреждение очень часто встречается моем коде. И оно тоже вызывает большие сомнения. Понятно, почему стоит этого опасаться… Мол не успеем инициализировать объект и будем что-то дёргать и все посыпется. Но PMD ругается этим предупреждением на любой метод класса, вызванный из конструктора, будь то сеттер или нечто другое (конечно, если метод не приватный). То есть, с их точки зрения, в конструкторах должна присутствовать только прямая инициализация внутренних параметров плюс весь сопутствующий код. А если этого кода много? А если он не один раз используется? В общем если неукоснительно следовать этому правилу, то это солидно усложняет жизнь.
4. Variable Naming Conventions
   Всем понятно, о чем речь и что неправильно называть переменные неправильно.:) Но бывают ситуации когда без этого тяжело обойтись. В моем случае чаще всего это оказывалось использование знака подчёркивания как префикса или суффикса. Но безусловно, этого нужно избегать.
5. Avoid reassigning parameters
   (имеется ввиду реинициализация входных параметров метода внутри метода)
   С моей точки зрения это не красиво. Действительно, лучше уж пользоваться временными локальными переменными. Прозрачнее код.
6. A class which only has private constructors should be final
   Ну и зачем? Что в одном, что во другом случае класс не наследуем. Что называется, одно другому не мешает. Может я что не понимаю?
7. Do not use the short type
   Ну тут тяжело не согласиться. Пользуется он только для экономии памяти, но при вычислениях все равно конвертится в int и обратно. Так что я вижу смысл использовать только константы этого типа.
8. Avoid instantiating Xxx objects...
   Где Xxx это String, Integer, Long, Boolean… У них у все есть valueOf(), а стрингу вообще ничего не надо. По большому счету — те же яйца, только в профиль. Единственное что, для обёрток числовых типов инициализация через valueOf() по их словам «is more memory friendly». Ну поверим.
9. Do not explicitly trigger a garbage collection
   Не считаю прямой вызов сборщика мусора хорошим тоном, но все таки иногда приходится.

Выводы

Ну в принципе полезная вещь, что бы по-вылавливать мелких тараканов из кода. Но не более чем. Я не могу сказать, что результаты работы плагина меня как-то удивили или расстроили. Просто стало ещё раз понятно, что работаем не без огрехов и что есть к чему стремиться.
Ещё хочется отметить, что есть возможность написания своих правил проверки кода. Лично попробовать не успел пока, но думаю там есть некоторое поле для фантазии.

Спасибо за внимание. Надеюсь эта статья была кому-то полезной.