Хитрости для рефакторинга фрагмента кода с множеством веток (если / то / еще) - PullRequest
4 голосов
/ 26 апреля 2009

Я с трудом пытаюсь реорганизовать некоторые фрагменты кода с множеством веток. Существует много блоков if / then / else, некоторые из которых являются вложенными.

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

Пока я в основном использую булеву алгебру (законы де Моргана). Я пытаюсь изменить условия в операторах if, чтобы я мог вызвать небольшие фрагменты кода вне блоков if / then / else. Это как-то помогает, но код все еще остается разветвленным. Я знаю, что в какой-то момент мне в конечном итоге придется разбить большие элементы на методы меньшего класса, но это сложно, потому что код содержит вызовы многих других методов класса и имеет много переменных локальной области видимости, поэтому мне придется иметь дело с передачей много аргументов в пользу новых методов. Интересно, смогу ли я использовать другие приемы для улучшения качества кода, прежде чем начинать разбивать его на отдельные более мелкие кусочки (методы класса).

Ответы [ 4 ]

14 голосов
/ 26 апреля 2009

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

не теряя много времени, пытаясь сначала понять все второстепенные аспекты функциональности?

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

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

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

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

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

Теперь, как это и произошло, это было в Sybase T-SQL, в котором есть глобальный для ошибок и глобальный для состояния курсора, которые он проверял дважды, один раз, когда он получил первую строку курсора, затем один раз в цикле, который повторяется по всем другим строкам. И он постоянно путал переменную ошибки (@@error) с переменной состояния курсора (@@sqlstatus, которая равна нулю при успехе (получил строку курсора), 1 при ошибке и 2, если в курсоре больше нет строк) , Его код работал по номинальному пути, потому что оба были равны нулю, если он получил строку, и когда он попытался получить строку после последней, он получил ошибку. Поэтому, если вы внимательно посмотрите на его код, вы будете стонать (еще раз!) И исправите это для него.

Но тогда вы посмотрите поближе и увидите, что он делал что-то вроде курсора в наборе всех строк where x = 1 и для каждого параметра строки y = y * 2, и вы в конечном итоге сказали бы ему: "просто напишите заявление об обновлении!

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

Выяснить это было сложнее, потому что вместо того, чтобы просто смотреть на локальное использование глобалов, вам пришлось искать два места: где был объявлен курсор (declare cursor_foo cursor for select * from table where x = 1 for update;) и двадцать строк вниз, где происходило обновление (update table set y = y * 2 where current of cursor_foo). (И все это будет многострочным, очень стандартным способом.)

Выяснить это было также сложнее, потому что вы просто предполагали, что никто не пройдет через все это только для обновления; конечно весь этот шаблон должен был быть, потому что в обновлении происходило что-то особенное, верно? Или потому что предикат where был динамическим или что-то в этом роде? Итак, вы посмотрите на это, и, будучи скромным программистом, который уважает своих коллег, ваш первый инстинкт был бы: «Что здесь пропущено I , должна быть причина курсор был использован? "

(Несмотря на то, что и я, и его начальник объясняли разницу между @@ error и @@ sqlstatus, он просто так и не понял, тем более, что он почти всегда мог просто сделать update; он думал с точки зрения процедурного код, и никогда не "получил" базы данных, несмотря на очевидный опыт "программиста базы данных".)

Урок заключается в том, чтобы не зацикливаться на мелочах, пока вы не подтвердите, что в первую очередь нужны мелочи (детали реализации). Разложив свой код в первую очередь, у вас будет больше шансов понять абстракции, с которыми вы имеете дело, и действительно улучшить код.

4 голосов
/ 26 апреля 2009

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

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

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

Удачи.

3 голосов
/ 26 апреля 2009

Прежде чем я начну рвать код, я создаю инфраструктуру модульного теста, чтобы убедиться, что я ничего не сломал. Затем я начинаю перемещать все дела в отдельные функции одну за другой. Код внезапно становится намного более читабельным, поскольку я пытаюсь дать понятные имена функциям, что также заставляет меня понимать код. Затем, как только это будет сделано, я начну смотреть на условия, чтобы увидеть, можно ли их также перефакторизовать, что часто возможно, поскольку огромное количество строк раньше могло затенить вещи. (Я думаю о функции, которую я унаследовал от кого-то, что было в основном 500 строками if-then-else)

0 голосов
/ 27 апреля 2009

Выясните, является ли ветвление основанным на таблицах или функциональности.

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

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...