Какие типы антишаблонов кодирования вы всегда подвергаете рефакторингу, когда пересекаете их?

Question

Я только что реорганизовал некоторый код, который находился в другом разделе класса, над которым я работал, потому что это была серия вложенных условных операторов (?:), которые стали намного понятнее благодаря довольно простому оператору переключения (C#).

Когда вы прикоснетесь к коду, который не является непосредственно тем, над чем вы работаете, чтобы сделать его более понятным?

Answer 1

Однажды я занимался рефакторингом и наткнулся на что-то вроде этого кода:

string strMyString;
try
{
  strMyString = Session["MySessionVar"].ToString();
}
catch
{
  strMyString = "";
}

Решарпер заметил, что .ToString() является лишним, поэтому я удалил его.К сожалению, это привело к нарушению кода.Всякий раз, когда MySessionVar имел значение null, это не вызывало исключение NullReferenceException, на которое опирался код, чтобы перенести его в блок catch.Я знаю, это был какой-то грустный код.Но я извлек из этого хороший урок.Не просматривайте старый код быстро, полагаясь на инструмент, который поможет вам выполнить рефакторинг — подумайте об этом сами.

В итоге я рефакторил его следующим образом:

string strMyString = Session["MySessionVar"] ?? "";

Обновлять: Поскольку за этот пост проголосовали и технически он не содержит ответа на вопрос, я решил, что мне действительно следует ответить на этот вопрос.(Ладно, это беспокоило меня до такой степени, что я действительно мечтал об этом.)

Лично я задаю себе несколько вопросов перед рефакторингом.

1) Находится ли система под контролем версий?Если да, продолжайте рефакторить, потому что вы всегда можете откатиться, если что-то сломается.

2) Существуют ли модульные тесты для функциональности, которую я изменяю?Если да, то отлично!Рефакторинг.Опасность здесь заключается в том, что существование модульных тестов не указывает на точность и объем упомянутых модульных тестов.Хорошие модульные тесты должны выявлять любые критические изменения.

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

В случае №3 я, вероятно, потратил бы время на то, чтобы фактически отследить весь код, который в настоящее время использует метод, который я рефакторингую.В зависимости от объема кода это может быть легко или невозможно (т.если это общедоступный API).Если речь идет о публичном API, вам действительно необходимо понять исходное предназначение кода с точки зрения бизнеса.

Answer 2

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

Answer 3

Это небольшой, незначительный антипаттерн, но он меня так раздражает, что всякий раз, когда я это нахожу, я немедленно удаляю это.На C (или C++ или Java)

if (p)
    return true;
else
    return false;

становится

return p;

В схеме,

(if p #t #f)

становится

p

и в МЛ

if p then true else false

становится

p

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

Answer 4

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

Answer 5

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

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

Answer 6

Это отличная ситуация, чтобы продемонстрировать преимущества модульных тестов.

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

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

Answer 7

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

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

• Удалите глупые комментарии
  • Комментарии, которые не говорят ничего больше, чем уже сказано в сигнатуре функции.
  • Комментарии, которые являются чистым идиотизмом, типа «вот как это выглядит».
  • Журналы изменений вверху файла (не зря у нас есть контроль версий)
  • Любые документы API, которые явно не синхронизированы с кодом.
• Удаление закомментированных фрагментов кода
• Добавьте теги контроля версий, например $Id$, если они отсутствуют.
• Исправьте проблемы с пробелами (однако это может раздражать других, потому что ваше имя отображается во многих строках в файле diff, даже если все, что вы сделали, это изменили пробелы)
  • Удалить пробелы в конце строк
  • Измените табуляцию->пробелы (поскольку это наше соглашение, в котором я работаю)

Answer 8

Если рефакторинг делает код много легче читать, для меня наиболее распространенным является дублирующийся код, например.if/else, который отличается только первой/последней командой.

if($something) {
  load_data($something);
} else {
  load_data($something);
  echo "Loaded";
  do_something_else();
}

Answer 9

Более (возможно) трех или четырех строк дублированного кода. всегда заставляет меня задуматься о рефакторинге.Кроме того, я склонен часто перемещать код, извлекая код, который, по моим прогнозам, будет использоваться чаще, в отдельное место — класс со своей четко определенной целью и обязанностями или статический метод статического класса (обычно размещаемый в мое пространство имен Utils.*).

Но, отвечая на ваш вопрос, да, во многих случаях сокращение кода не обязательно означает, что он должен быть хорошо структурирован и удобочитаем.Используя ?? оператор в C# — еще один пример.Вам также следует подумать о новых функциях выбранного вами языка, например:LINQ можно использовать для выполнения некоторых задач в очень элегантная манера, но также может сделать очень простую вещь нечитаемой и слишком сложный.Вам нужно очень тщательно взвесить эти две вещи, в конечном итоге все сводится к вашему личному вкусу и, в основном, опыту.

Ну, это еще один ответ «это зависит», но я боюсь, что так и должно быть.

Answer 10

Я почти всегда разбиваю >2 одинаковых условий на переключатель...чаще всего в отношении перечислений.Я буду сокращать возврат вместо длинного заявления.

бывший:

if (condition) {
  //lots of code
  //returns value
} else {
  return null;
}

становится:

if (!condition)
  return null;

//lots of code..
//return value

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

Answer 11

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

Но иногда ReSharper указывает на некоторые вещи, перед которыми я просто не могу устоять, нажав Alt+Enter.:)

Answer 12

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

Это бесконечно повышает читаемость.

Answer 13

Я бы реорганизовал только тот код, с которым столкнулся и над которым не работал активно, после выполнения следующих шагов:

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

Answer 14

Рефакторинг ради него — один из корней всех зол.Пожалуйста, никогда не делай этого.(Модульные тесты несколько смягчают это).

Answer 15

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

Answer 16

Я пытаюсь провести рефакторинг, основываясь на следующих факторах:

1. Достаточно ли я понимаю, чтобы понять, что происходит?
2. Могу ли я легко вернуться назад, если это изменение нарушит код.
3. Хватит ли у меня времени, чтобы отменить изменение, если оно нарушит сборку.

И иногда, если у меня есть достаточно времени, я занимаюсь рефакторингом, чтобы научиться.Например, я знаю, что мое изменение может нарушить код, но я не знаю, где и как.Поэтому я все равно меняю его, чтобы выяснить, где он сломался и почему.Таким образом я узнаю больше о коде.

Моей сферой деятельности было мобильное программное обеспечение (сотовые телефоны), большая часть кода которого находится на моем компьютере и не влияет на работу других.Поскольку я также поддерживаю систему сборки CI для своей компании, я могу запустить полную сборку продукта (для всех телефонов) на основе рефакторингового кода, чтобы убедиться, что он ничего не сломает.Но это моя личная роскошь, которой у вас может не быть.

Answer 17

Я склонен довольно часто рефакторить глобальные константы и перечисления, если их можно считать низким риском рефакторинга.Возможно, это просто так, но что-то вроде перечисления ClientAccountStatus должно находиться в классе ClientAccount или близко к нему, а не в классе GlobalConstAndEnum.

Answer 18

Удаление/обновление комментариев, которые явно ошибочны или явно бессмысленны.
Удаление их заключается в следующем:

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

Это единственный известный мне рефакторинг со 100% безрисковым подходом.

Обратите внимание, что делая это в структурированный комментарии, подобные комментариям Javadoc, — это другое дело.Их изменение сопряжено с риском, поэтому они очень вероятный быть исправлены/удалены, но не мертвы, некоторые гарантированные исправления, как это было бы со стандартными неправильными комментариями к коду.