Как бы вы реорганизовали этот код?

Question

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

if Condition1=true then
begin
  //loop through every cell in row, 
  //if aCell/headerCellValue>1 then
  //color aCell red
end
else if Condition2=true then
begin
  //do some other calculation adding cell and headerCell values, and
  //if some other product>2 then
  //color the whole row green
end
else show an error message

Я смотрю на это и говорю: «Ах, рефакторинг паттерна стратегии! Код будет легче понять, легче отладить и потом будет легче расширять!» Я понял это.

И я могу легко разбить код на несколько процедур.

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

Я очень жду ответов. Спасибо, как всегда - Эл С.

Answer 1

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

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

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

if Condition1 then
  ColorCellsRedAboveRatio(Grid, 1.0)
else if Condition2 then
  ColorRowsGreenAboveProduct(Grid, 2)
else
  Error;

Answer 2

Прежде всего, НИКОГДА не сравнивайте значение с истинным. Это

if Condition1 then
begin
end else if Condition2 then
begin
end;

Сравнение с истиной в худшем случае может дать сбой, даже если значение равно истина. На форумах сообщества Delphi-PRAXiS.net есть хорошая (но немецкая) статья «Über den Umgang mit Boolean», в которой приведен пример, воспроизводящий это странное поведение.

Относительно вашего вопроса напрямую:

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

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

Answer 3

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

Итак, если вы обнаружите, что у вас много кода, в котором есть несколько сотен условий if ... else в обработчиках событий, как я часто это делаю, я на минуту забуду об этом обработчике событий, и уменьшите его, как сказал человек выше, до минимальной объектно-ориентированной модели:

если тогда DOA (меньше, параметры) иначе, если б то DOB (это, вообще говоря, лучше) иначе, если с, то DOc; ...

Теперь, если doA, doB и doC принадлежат друг другу в другом объекте (они разделяют состояние и изменяют / контролируют некоторый конкретный набор полей), то я мог бы переместить методы doA, doB, doC в другой объект.

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

procedure TForm1.BigGuiControlRightButtonClick(Sender;...);
begin
     BigThingController.RightClickMenuHandler(Sender, ....)
end;

procedure TForm1.BigGuiControlDoSomeThing(Sender:TObject);
begin
    BigThingController.DoSomeThing;
end;

procedure TForm1.Print(Sender);
begin
     DocumentManager.Print(Document);
end;

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

Answer 4

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

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