18 октября 2011 в 19:10

Фашизм в коде. Часть вторая из песочницы

Java*
imageВ своем предыдущем посте мой коллега попытался расскрыть идею положительного влияния "фашизма" в коде на примере одного из проверочных модулей (чеков). Вместе с примером была предоставлена наша сборка плагина с некоторым расширением. Наша команда разработала ряд новых чеков и упростила установку в Eclipse.

«Фашизм» в коде посредством использования проекта Checkstyle… Что может быть лучше такой идеологии? Половина, а то и больше замечаний, которые делаются при обычном ревью кода программистами может быть исправленна (недопущена) еще на этапе написания кода. Применение проверочных модулей оказывает огромное влияние на внешний вид и читабельность, а значит, и поддерживаемость программ. Существует масса книг в которых авторы делатся опытом как не надо писать код и показывают самые популярные ошибки. Все уважающие себя программисты читали такие книги как «Effective Java» J.Bloch, «Java Puzzlers» J.Bloch, Java Pitfalls и …. Но все ли советы вы используете из них? Как показывает практика даже серьезные программисты допускают банальные ошибки.

После первого релиза, мы решили упростить процесс использования наших наработок. Решением было организовать свой проект с чеками, невошедшими в релиз Checkstyle, в виде дополнения (feature) к плагину Checkstyle для Eclipse. Упрощенный процесс установки свелся к:
  1. Ставим плагин плагин Checkstyle (update-site: eclipse-cs.sourceforge.net/update ), если он еще не стоит.
  2. Ставим наше дополнение (update-site: sevntu-checkstyle.github.com/sevntu.checkstyle/update-site ).

Получаем в настройках чекстайла группу с нашими проверками:

image
Рисунок 1 – Диалоговое окно конфигурации проверочных модулей (чеков)

После установки нашего модуля он появляется в списке существующих под именем "SevNTU checks". Таким образом, не дожидаясь основного релиза Checkstyle, мы можем полноценно использовать наши чеки для автоматического контроля качества Java-кода. Вкратце опишу последние из них.

Проверочный модуль "ForbidAnnotationCheck" – производит проверку на использование аннотаций, указанных в конфигурации чека. Например:

@Autowired
private DataSource dataSource;


С помощью этого чека можно запретить @Autowired на полях класса, и заставить программиста не поленитья и воспользоваться сеттером для @Autowired.

Проверочный модуль "VariableDeclarationUsageDistanceCheck" – используется для проверки расстояния между определением переменной и ее первым использованием. Если расстояние превышает некоторое, изначально заданное, значение, то пользователь информируется сообщением. Проверка производится для случая, когда определение переменной и ее использование находятся в одной области видимости. Включает следующие опции:
  1. allowedDistance – установка допустимого расстояния между определением переменной и ее первым использованием.
  2. ignoreVariablePattern – установка регулярного выражения для имен переменных, исключаемых из проверки.
  3. validateBetweenScopes – проверка расстояния для случая, когда определение переменной и ее использование находятся в разных областях видимости.
  4. ignoreFinal – игнорирование переменных, в определении которых используется ключевое слово final.

Проверочный модуль "AbbreviationAsWordInNameCheck" – производит проверку имен классов, интерфейсов, переменных и методов на нахождение в них аббревиатур с заглавными буквами. Пример: «XMLReader» должен быть «XmlReader». Включает следующие опции:
  1. allowedAbbreviationLength – разрешимое число идущих подряд заглавных букв в аббревиатуре. Например: "interface ABCDInterface". Здесь аббревиатура "ABCD" и слово "Interface". Для того, чтобы данный случай игнорировался, в данной опции следует поставить число идущих подряд заглавных букв в аббревиатуре, равное 4.
  2. allowedAbbreviations – список аббревиатур, которые будут игнорироваться.
  3. ignoreFinal – игнорирование переменных, в определении которых используется ключевое слово final.
  4. ignoreStatic – игнорирование полей, в определении которых используется ключевое слово static.

Проверочный модуль "OverridableMethodInConstructorCheck" – производит проверку на вызов переопределенных (overridable) public/protected методов из конструктора, readObject and clone мктодов класса. Проверяется весь стек вызовов, например, constructor -> private method -> public/protected method.

Проверочный модуль "ReturnNullInsteadOfBoolean" – служит для отслеживания ситуаций, когда в декларации метода тип возвращаемого объекта – Boolean, но сам метод может вернуть null.

Проверочный модуль "AvoidNotShortCircuitOperatorsForBooleanCheck" – производит проверку условных выражений на использование bitwise операторов ("|", "&", "|=", "&=") вместо обычных для булевого типа операторов short-circuit логики.

Проверочный модуль "AvoidConstantsInInterfacesCheck" – константы следует определять в классах (см. J.Bloch «Effective Java» Item 19: Use interfaces only to define types).

Проверочный модуль "AvoidHidingCauseExceptionCheck" – информирует пользователя о том, что тот скрыл причину исключительной ситуации. Например:

catch (IllegalStateException e) {
// your code
throw new RuntimeException();
}


В данном фрагменте кода происходит скрытие исключения "IllegalStateException".

Проверочный модуль "IllegalCatchExtendedCheck" – производит проверку блоков catch() на захват исключений типа java.lang.Exception, java.lang.Error или java.lang.RuntimeException. Включает следующие опции:
  1. allowThrow – игнорирование блока catch(), если в нем выбрасывается новое исключение.
  2. allowRethrow – игнорирование блока catch(), если в нем выбрасывается отловленное исключение.

Проверочный модуль "ReturnBooleanFromTernary" – производит проверку ветвей тернарного оператора. При нахождении 'true' или 'false', как единственного содержимого ветки, информирует пользователя сообщением. Например, вместо строки "boolean b = a ? false : true;" лучше использовать "b = !a;".

Если вы считае эти проверки прописными истинами, которые все знают – проверьте свой код и вы будете удивлены!!!

Кого заинтересовала данная тема, да и кому интересно опробовать наши чеки на своих проектах, приглашаю на сайт проекта. Wiki заметки «в помощь разработчику» здесь.

Благодарю за внимание!

P.S. FindBug, PMD, Sonar… мы знаем о существовании этих продуктов, и некоторые идеи были заимствованы из них. Но по ряду причин мы сфокусировали свои силы имеенно на Сheckstyle.
Ruslan Diachenko @rusya7
карма
5,0
рейтинг 0,0
Похожие публикации
Самое читаемое Разработка

Комментарии (25)

  • +4
    Молодцы!
    • +1
      однозначно!
  • +5
    А я правильно понимаю, что в Intellij Idea это в принципе все встроено по-умолчанию?
    • 0
      Возможно, но мы используем Eclipse, так как он является стандартом для нашей команды.
      • +3
        Вот это действительно фашизм — заставлять всех людей пользоваться одной средой разработки.
        • 0
          Когда в команде все согласны — это не проблема. Настаиваю на том, что провеки представлены в плагине лишь для удобства. Их легко можно переделать, как расширение в checkstyle-maven-plugin и счатье будет всем, кто использует maven. Опять фашизм на сонове билд машины :) Фашизм можно найти везде, поэтому так и назвали данную статью. А как вы воспользуетесь нашими наработками, это уже дело ваше. Мы надеемся, что вышепересиленные проверки попадут в общий релиз Checkstyle, и тогда сачтье будет всем.
    • 0
      www.jetbrains.com/idea/documentation/inspections.jsp
      Idea мееет похвальный список проверок, но все же НЕ все там есть:
      ForbidAnnotationCheck
      VariableDeclarationUsageDistanceCheck
      AbbreviationAsWordInNameCheck

      Более того, проверки в Идее, насколько я понимаю, не могут быть использованы в билдах и другими тулзами: maven (for maven report site) and Sonar,…

      Наличие в Идее встроенной поддержки DSM не помешало нам начать разработку мавен плагина для этого
      способа анализа зависимомтей (см. наш репозорий), но об этом уже в другой статье.

      In Idea:
      AvoidNotShortCircuitOperatorsForBooleanCheck
      AvoidConstantsInInterfacesCheck
      AvoidHidingCauseExceptionCheck
      IllegalCatchCheck
      OverridableMethodInConstructorCheck
      ReturnBooleanFromTernary
      ReturnNullInsteadOfBoolean
  • –2
    а чем это лучше codestlye в билд скрипте? всё равно же билд навернётся, что, в принипе, и является конечной целью?
    • +2
      Это просто другой, более удобный подход, использование которого позволяет исправлять код еще НА ЭТАПЕ его написания!!! Но если вам удобен процесс: построить проект > взять файл, найти и исправить в нем warning сообщения > построить проект > взять файл… то вы можете взять jar файл с нашего проекта и использовать его в своем билд скрипте. Проект с чистым jar-ом называется checkstyle-sevntu. Проект checkstyle-sevntu-plugin используется для плагина в Eclipse.
      • 0
        грош цена такому стилю, который надо постоянно исправлять. К принятому стилю надо привыкнуть за пару дней, а потом уже в одном и том же стиле код колбасить. А вот разработчикам, которые ему не следуют, надо создать максимум неудобств, чтобы писали правильно :)
        • +1
          Плагин помогает команде создать и контролировать стиль, подсвечивать проблемные участки и исправлять код. Смена стиля — это уже проблемы команды, а не утилит для его проверки.
          • –1
            а я контроль и имел ввиду. Смена — это для вновь пришедшего в команду
            • 0
              Стиль — не религия, и бывает такое, что новые люди привносят новые интресные случаи, которые приводят к разногласия в команде. Стиль можно дополнить также, как добавляются правила в Code Standard, если это не было оговорено и как бы очевидным это не казалось.
              Пример: использование меток (goto). Новый человек использовал их для якобы оптимизации и почти обоснованно. Но заметить это не сразу можно. Когда стали разбираться — решили вопрос без меток, метки забанили Чевстайлом — изменение в стиле.
              • –1
                стиль — это не религия, это стандарт. Если такого стандарта нет, то и checkstyle не нужен. И, поверьте, в такоп проетке потом ковыряться — одно «удовольствие»…

                Какие бы интересные случаи не были в конце концов стандарт утрясётся в каком-либо виде и новые разработчики доkжны прописанным стандартам и следовать.
                • 0
                  А есть ли у вас полный стандарт для кода, полностью прописаный по всем контрукциям классов? И где гарантия того, что новый разработчик сразу поймет и запомнит его полностью? А править и делать code review регулярно — у вас, возможно, не будет времени на то, что можно автоматизировать. Опыт увсех разный, и, частенько, даже опытные программсты допускают ошибки или просто забывают в суете фиксов.
                  • –1
                    а зачем покрывать ВСЕ кейзы стандартом? Документ должен быть простым для прочтения и понимания.

                    И именно что бы в ходе code review не тыкать пальцем в скобки и нужен стандарт. Цель ревью — проверка написанного функционала, хотя би и поверхностная. И если времени на ревью не хвататает — оно потом отлично находится на последующий багфиксинг.

                    А еще «опытные» программисы в суете фиксят баг и коммитят его. И только потом запускают регрессионные тесты. А вот потом начинается самое интересное…

                    А нормальному разработчику неверный codestyle глаз режет, потому что начинаешь задумываться, а что этим тут хотели сказать?
                    • 0
                      То что нормальному программисту глаз режит — то еще не совсем нормальному программисту на глаз приятно и очень удобно кодируется :).
                      Я рад за вас что у вас таких нет в команде, но мир не идеален.
                      • 0
                        вот таких ненормальных надо переучивать, а не жизнь облегчать :)
                        • 0
                          Зачем же тратить время на переучивание, если можно просто дать использовать Чекстайл. Он сам все подскажет, где нужно исправить код, как лучше писать. Это и является одной из его целей: показать разработчику, как нужно писать код правильно, и заставить его следовать правилам, описанным в чеках. Как раз в таком подходе жизнь облегчается не для «ненормального», а для всей команды.

                          Переучиванием занимается Чекстайл: грамотному подсказывает, несовсем грамотному показывает, что ему нужно прогулить, а упрямца ловит сразу на месте, и можно сконфигурировать Checkstyle так, что бы он давал ERROR, тем самым ломая упрямцу локальную компиляцию — он начнет возмущаться и вот тут-то вы ему все и объясните.

                          Профит — экономия времени, нервов.
      • –1
        Кстати, а вот PMD или Findbugs во встроенном режиме в эклипесе — это уже гораздо лучше. Ждём с нетерпением :)
        • 0
          Данные средства решают разные задачи. В некоторых моментах они пересекаются с Checkstyle. Мы их также используем, так как они дополняют друг друга.
        • 0
          Используйте Sonar plugin для Eclipse в нем и PMD и FindBugs. Может проверки запускать и локально и удаленно.
          • 0
            Спасибо, используем уже давно, но проверки эти мы делаем для того, что бы не дожидатся релиза Checkstyle и еще потом релиза от Sonar. Но все модули высылаются как патчи в основной проект — не все чеки принимаются, и ждать мы не можем долго.
  • –5
    Пересел неделю назад с Java на Erlang! Все время радуюсь!
    • 0
      Угу, дойдете до написание бложика или чего-то подобного и поймете, что там даже ORM-а нет :)

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