«Косметическая» очистка старого, неизвестного кода. Какие шаги, какой порядок? Насколько агрессивен? - PullRequest
7 голосов
/ 23 сентября 2010

Когда я получаю код, который раньше не видел, чтобы перевести его в некое нормальное состояние, я обычно исправляю «косметические» вещи (например, преобразование StringTokenizers в String#split(), замену коллекций до 1.2 на более новые коллекции, создание полей final, преобразование массивов в стиле C в массивы в стиле Java, ...) при чтении исходного кода, с которым мне нужно ознакомиться.

Много ли людей используют эту стратегию (может быть, это некая «лучшая практика», которую я не знаю?) Или это считается слишком опасным, и обычно предпочтительнее не трогать старый код, если он не является абсолютно необходимым? Или более часто объединяют этап «косметической очистки» с более инвазивным этапом «общего рефакторинга»?

Каковы общие "низко висящие фрукты" при выполнении "косметической очистки" (в отличие от рефакторинга с более инвазивными изменениями)?

Ответы [ 11 ]

7 голосов
/ 23 сентября 2010

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

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

Обычно я начинаю с просмотра кода, который часто используется и, во-первых, его нужно будет часто менять. (Это оказывает наибольшее влияние за наименьшее время ...) Имена переменных, пожалуй, самый простой и безопасный «низко висящий фрукт» для атаки вначале, за которым следуют обновления фреймворка (изменения коллекции, обновленные методы и т. Д.). После того, как это сделано, обычно я разбираюсь с большими методами, после чего следуют другие типичные рефакторинги.

4 голосов
/ 23 сентября 2010

Здесь нет правильного или неправильного ответа, поскольку это во многом зависит от обстоятельств.

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

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

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

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

3 голосов
/ 23 сентября 2010

Самый низко висящий косметический фрукт (в любом случае в Eclipse) shift-control-FАвтоматическое форматирование - твой друг.

2 голосов
/ 24 сентября 2010

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

Кроме того, все делают ошибки. Каждое нетривиальное изменение, которое вы делаете (изменение StringTokenizer на split - это , а не автоматическая функция, например, в Eclipse, так что вы пишете это самостоятельно) - это возможность появления ошибок. условный, или вы просто по ошибке забыли !?

Следовательно, ваши изменения подразумевают повторное тестирование. Эта работа может быть весьма существенной и серьезно сокрушить небольшие изменения, которые вы сделали.

2 голосов
/ 23 сентября 2010

Я не изменяю старый код, кроме как переформатировать его с помощью IDE. Существует слишком большой риск введения ошибки - или удаления ошибки, от которой теперь зависит другой код ! Или введение несуществующей зависимости, например использование кучи вместо стека.

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

Если босс попросит меня исправить ошибку в коде, я внесу как можно меньше изменений. Скажем, ошибка в простом цикле for. Я бы реорганизовал цикл в новый метод. Затем я бы написал тестовый пример для этого метода, чтобы продемонстрировать, что я обнаружил ошибку. Тогда я бы исправил новый метод. Тогда я бы удостоверился, что тесты прошли успешно.

Да, я подрядчик. Заключение контракта дает вам другую точку зрения. Я рекомендую это.

2 голосов
/ 23 сентября 2010

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

Я всегда (да, верно ... иногда есть что-тоназывай крайний срок, который меня смущает) начни с такого подхода, и до сих пор он мне очень помог.

2 голосов
/ 23 сентября 2010

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

Запустите такой инструмент, как JDepend , CheckStyle или PMD на источнике.Они могут автоматически выполнять множество изменений, которые являются общими, но основаны на общих правилах рефакторинга.

2 голосов
/ 23 сентября 2010

Вы можете купить копию Рефакторинг: Улучшение дизайна существующего кода у Мартина Фаулера, вы найдете много вещей, которые вы можете сделать во время операции рефакторинга.

Кроме того, вы можете использовать инструменты, предоставляемые вашей IDE и другими анализаторами кода, такие как Findbugs или PMD для обнаружения проблем в вашем коде.


Ресурсы:

На ту же тему:

2 голосов
/ 23 сентября 2010

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

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

Дальнейший рефакторинг может быть выполнен более безопасно и локально.

1 голос
/ 23 сентября 2010

От опыта зависит две вещи: время и риск.

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

Если код, над которым вы работаете, имеет много зависимостей или имеет критическое значение для приложения, внесите как можно меньше изменений - вы никогда не знаете, что может сломать ваше fix ...:)

Звучит так, будто у вас есть четкое представление о том, как должны выглядеть , поэтому я не собираюсь говорить, какие конкретные изменения нужно внести в каком порядке, потому что это будет варьироваться от человека к человеку. Сначала внесите небольшие локализованные изменения, протестируйте, расширьте сферу ваших изменений, протестируйте. Expand. Тестовое задание. Expand. Тестовое задание. Пока у вас не останется времени или не останется места для улучшений!

Кстати, при тестировании вы, скорее всего, увидите, где что-то ломается чаще всего, - создайте для него тестовые случаи (JUnit или что-то еще).

ИСКЛЮЧЕНИЕ: Две вещи, которые я всегда делаю, это переформатирование (CTRL + SHFT + F в Eclipse) и комментирование кода, который не очевиден. После этого я сначала забиваю самый очевидный гвоздь ...

...