Должен ли я рефакторинг больших функций, которые в основном состоят из одного регулярного выражения? [закрыто]

15

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

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

DudeOnRock
источник
3
Я не думаю, что с вашей функцией что-то не так, учитывая, что большая ее часть - документация . Однако может возникнуть проблема с удобством обслуживания при использовании большого регулярного выражения.
Джоэл Корнетт
2
Вы уверены, что гигантское регулярное выражение - лучшее решение вашей проблемы? Рассматривали ли вы более простые альтернативы, такие как библиотека синтаксического анализатора или замена пользовательского формата файла стандартным (XML, JSON и т. Д.)?
Лортабак
2
Существуют ли другие функции, использующие измененную / улучшенную / упрощенную версию этого регулярного выражения? Это было бы важным показателем того, что рефакторинг должен иметь место. Если нет, я бы оставил все как есть. Необходимость сложной манипуляции со строками, подобной этой, - это желтый флаг сам по себе (ну, я не знаю контекста, а значит, просто желтый), и рефакторинг функции внизу кажется мне скорее ритуалом избавления от чувства вины, которое испытывают люди. это;)
Конрад Моравский
8
Как регулярное выражение из 100 строк может сделать только 1 вещь?
Питер Б
@lortabac: Вводимый пользователем текст (проза.)
DudeOnRock,

Ответы:

36

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

Вы четко сделали свою домашнюю работу:

  • Цель функции понятна.
  • Принципы его реализации понятны (то есть читаемы).
  • Есть полные тесты реализации.
  • Эти тесты пройдены, что означает, что вы считаете, что реализация верна.

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

Второе голосование происходит от просмотра ваших вариантов и того, что вы получаете (и теряете) от каждого:

  • Рефакторинг. Это дает вам согласие с чьим-то представлением о том, какой длины должна быть функция, и жертвует читабельностью.
  • Ничего не делать. Это поддерживает существующую читаемость и жертвует согласием с чьим-то представлением о том, какой длины должна быть функция.

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

Итог: если он не сломан, не чините его.

Blrfl
источник
10
+1 за "Если это не сломано, не исправляйте это."
Джорджио
В самом деле. Правила Сэнди Мец ( gist.github.com/henrik/4509394 ) хороши и все, но на youtube.com/watch?v=VO-NvnZfMA4#t=1379 она рассказывает о том, как они появились и почему люди принимают они слишком серьезно.
Амадан
@Amdan: с дополнительным контекстом из видео, что сделал Мец, имеет смысл. Ее рекомендация одному клиенту была преднамеренно экстремальной с одной стороны, чтобы противостоять поведению, которое с другой стороны было экстремальным, как способ перетянуть его в более разумную середину. Остальная часть этой дискуссии сводится к сути моего ответа: рассуждения, а не вера, являются способом определения наилучшего курса действий.
Blrfl
19

Честно говоря, ваша функция может «делать одно», но, как вы сами заявили

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

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

но я чувствую, что таким образом я теряю читабельность, так как я эффективно переключаю языки

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

  • покажите свой код другому продвинутому разработчику (может быть, на /codereview// ), чтобы другие думали о читабельности так же, как вы. Будьте открыты к мысли, что другие могут не найти 100-строчный регистр, такой же читаемый, как вы. Иногда понятие «его нелегко разбить на мелкие кусочки» можно преодолеть с помощью второй пары глаз.

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

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

Док Браун
источник
Я бы сказал, что именованные группы захвата (вообще, группы захвата) наиболее похожи на переменные final / write-Once или, возможно, макросы. Они позволяют ссылаться на определенные части соответствия, либо из объекта соответствия, возвращенного процессором регулярных выражений, либо позже в самом регулярном выражении.
JAB
4

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

Длина - это произвольный «стандарт» в отношении размера кода. Если функция на 100 строк в C # может считаться длинной, в некоторых версиях сборки она будет крошечной. Я видел несколько запросов SQL, которые были в пределах 200 строк кода, которые возвращали один очень сложный набор данных для отчета.

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

Не меняйте его только потому, что оно длинное.

Адам Цукерман
источник
3

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

my $start_re = qr/(?:\w+\.\w+)/;
my $middle_re = qr/(?:DOG)|(?:CAT)/;
my $end_re = qr/ => \d+/;

my $final_re = $start_re . $middle_re . $end_re;
# or: 
# my $final_re = qr/${start_re}${middle_re}${end_re}/
Рори Хантер
источник
Я использую подробный флаг, который даже удобнее, чем вы предлагаете.
DudeOnRock
1

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

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

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

arfo
источник
1

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

Томас Эдинг
источник
Я думал об этом сам. Проблема заключается в том, что ввод является прозой, и я беру подсказки из контекста и форматирования. Если есть возможность написать парсер для чего-то подобного, я бы хотел узнать об этом больше! Я не мог ничего найти сам.
DudeOnRock
1
Если регулярное выражение может проанализировать его, вы можете проанализировать его. Ваш ответ заставляет меня думать, что вы не очень хорошо разбираетесь в разборе. Если это так, вы можете придерживаться регулярного выражения. Либо так, либо выучите новый навык.
Томас Эдинг
Я хотел бы выучить новый навык. Любые хорошие ресурсы, которые вы можете предложить? Меня также интересует теория, стоящая за этим.
DudeOnRock
1

Гигантские регулярные выражения - плохой выбор в большинстве случаев. По моему опыту, они часто используются, потому что разработчик не знаком с анализом (см . Ответ Томаса Эдинга ).

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

Поскольку я не знаю реального кода, я рассмотрю два возможных сценария:

  • Регулярное выражение простое (много буквального соответствия и мало альтернатив)

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

  • Регулярное выражение сложное (много альтернатив)

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

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

lortabac
источник