Java: Лучшие практики для превращения иностранного кода ужасов в чистый API ...? - PullRequest
4 голосов
/ 21 марта 2010

У меня есть проект (связанный с графовыми алгоритмами). Написано кем-то другим.

Код ужасен:

  • открытые поля, нет методов получения / установки
  • огромные методы, все публично
  • некоторые классы имеют более 20 полей
  • некоторые классы имеют более 5 конструкторов (которые также огромны)
  • некоторые из этих конструкторов просто оставляют много полей null
    (поэтому я не могу сделать некоторые поля окончательными, потому что тогда каждый второй конструктор сигнализирует об ошибках)
  • методы и классы опираются друг на друга в обоих направлениях

Я должен переписать это в чистый и понятный API.

Проблема в том, что я сам ничего не понимаю в этом коде.

Пожалуйста, дайте мне советы по анализу и пониманию такого кода.

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

Ответы [ 10 ]

8 голосов
/ 22 марта 2010

О, дорогой :-) Я завидую тебе, а не в одно и то же время ... ладно, давайте рассмотрим одну вещь за раз. Некоторые из этих вещей вы можете решить сами, прежде чем использовать инструмент анализа кода. Таким образом, вы получите лучшее понимание и сможете продвинуться гораздо дальше, чем с помощью простого инструмента

  • открытые поля, нет методов получения / установки
    • сделай всё приватным. Ваше правило должно быть максимально ограничить доступ
  • огромные методы, все публично
    • разделить и сделать приватным, где это имеет смысл
  • некоторые классы имеют более 20 полей
    • тьфу ... шаблон Builder в Effective Java 2nd Ed - главный кандидат на это.
  • некоторые классы имеют более 5 конструкторов (которые также огромны)
    • Похоже на телескопические конструкторы, тот же шаблон, что и выше, поможет
  • некоторые из этих конструкторов просто оставили множество полей пустыми
    • да, это телескопические конструкторы:)
  • методы и классы опираются друг на друга в обоих направлениях
    • Это будет наименее весело. Попробуйте удалить наследство, если вы не совсем ясно, это требуется и использовать композицию вместо этого через интерфейсы, где это применимо

Удачи, мы здесь, чтобы помочь

6 голосов
/ 22 марта 2010

WOW!

Я бы порекомендовал: написать юнит-тесты, а затем начать рефакторинг

* public fields, no getters/setters

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

* huge methods, all public

понять их семантику, попробуйте ввести интерфейсы

* some classes have over 20 fields

очень часто встречается в сложных приложениях, не о чем беспокоиться

* some classes have over 5 constructors (which are also huge)

замените их на шаблон покупателя / создателя

* some of those constructors just left many fields null

см. Ответ выше

* methods and classes rely on each other in both directions

решить, стоит ли переписывать все (честно говоря, я сталкивался с делом, где нужен был только 10% кода)

3 голосов
/ 22 марта 2010

Ну, мастер очистки в затмении соскребет заметный процент от осадка.

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

2 голосов
/ 22 марта 2010

Используйте IDE, которая знает что-то о рефакторинге, например IntelliJ. У вас не будет ситуаций, когда вы перемещаете один метод, а пять других классов жалуются, потому что IntelliJ достаточно умен, чтобы внести все необходимые изменения.

Модульные тесты обязательны. Кто-то рефакторинг без юнит-тестов похож на проводника без защитной сетки. Получить его, прежде чем начать длинный, тяжелый подъем.

2 голосов
/ 22 марта 2010

Для статического анализа и графиков вызовов (без графики, кроме структур графиков) вы можете использовать Поиск зависимостей .

1 голос
/ 22 марта 2010

Не говоря о таких инструментах, как Sonar, FindBugs и т. Д., О которых некоторые уже упоминали, они не помогают, но здесь нет никаких фокусов.Начните с того, что вы понимаете, создайте для него модульный тест, и, как только он запустится, начните рефакторинг по частям.Не забывайте высмеивать зависимости по ходу дела.

1 голос
/ 22 марта 2010

Вот как бы я это сделал:

  1. Начните использовать код, например, из main метода, как если бы он использовался другими классами - те же аргументы, те же порядки вызова Сделайте это внутри отладчика, как вы видите каждый шаг, который делает этот класс.
  2. Начните писать модульные тесты для этой функциональности. Когда вы достигнете разумного покрытия, вы начнете замечать, что у этого класса, вероятно, слишком много обязанностей.

while ( responsibilities != 1 ) {

  1. Извлечение интерфейса, который выражает одну ответственность этого класса.
  2. Заставить всех абонентов использовать этот интерфейс вместо конкретного типа;
  3. Извлечь реализацию в отдельный класс;
  4. Передать новый класс всем вызывающим абонентам, используя новый интерфейс.

}

1 голос
/ 22 марта 2010

Ответ может быть: терпение и кофе.

0 голосов
/ 26 января 2012

Хотя некоторый унаследованный код может быть едва понятным, тем не менее он может быть подвергнут рефакторингу и улучшен до разборчивого уровня. Вы видели книгу Джошуа Кериевского Refactoring to Patterns ? - это хорошо.

0 голосов
/ 22 марта 2010

Иногда проще что-то переписать с нуля. Этот «ужасный код» работает как задумано или полон ошибок? Это задокументировано?

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

...