Замените Temp на Query

Question

Метод рефакторинга Replace Temp with Query сейчас довольно широко рекомендуется, но кажется очень неэффективным для очень небольшого выигрыша.

Метод с сайта Мартина Фаулера приводит следующий пример:

Извлечь выражение в метод. Замените все ссылки на temp на выражение. Затем новый метод можно использовать в других методах.

    double basePrice = _quantity * _itemPrice;
    if (basePrice > 1000)
        return basePrice * 0.95;
    else
        return basePrice * 0.98;

становится

    if (basePrice() > 1000)
        return basePrice() * 0.95;
    else
        return basePrice() * 0.98;


double basePrice() {
    return _quantity * _itemPrice;
} 

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

Я что-то упустил?

Answer 1

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

    double basePrice = basePrice();
    if (basePrice > 1000)
            return basePrice * 0.95;
    else
            return basePrice * 0.98;

Фаулер не дает объяснения, почему его пересмотренный код лучше оригинала, кроме как читать его книгу.

Answer 2

Я перечитываю Refactoring прямо сейчас, и одной из причин является следующая цитата в конце Extract Method p.116.

Справочная информация: он описывает, что Extract Method трудно, если код для извлечения использует локальные переменные.

Временные переменные часто бывают настолько многочисленными, что делают извлечение очень неудобным. В этих случаях я пытаюсь уменьшить температуру, используя Replace Temp with Query (120). ...

Таким образом, один рефакторинг может зависеть от первого выполнения другого рефакторинга, что само по себе не очевидно. Другой пример - Inline Method перед заменой Method with Method Object.

Answer 3

Это рефакторинг, который важен, так как это рефакторинг с единственной ответственностью, это решение СУХОГО сбоя!

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

Нет тестов, этот метод имеет множество зависимостей - какой беспорядок! :)

Снять временную шкалу о рефакторинге в направлении единоличной ответственности.

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

Причина, по которой мой метод содержит временную переменную, заключается в том, что он выполняет работу, которую не должен ... и эта логика повторяется аналогичными классами ... все ли это согласованно? НЕТ!

Удалив temp, я также реорганизую код в класс с соответствующей ответственностью, которая может быть покрыта на 110% несколькими простыми тестами.

Нет огромного приспособления для тестирования чего-то тривиального

Если я назову что-нибудь дорогое, я верну результат в виде объекта значения / агрегата. Вероятно, «временный код» должен быть усвоен агрегатом.

Так что удаление temp подталкивает вас к централизованной (единоличной) ответственности -> SOLID!

Answer 4

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

Answer 5

В книге «Рефакторинг» есть что-то еще о «Заменить темп на запрос», и это:

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

Answer 6

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

  if (basePrice() > 1000)
     return bigTicketDiscount()
  else
     return regularDiscount()

double bigTicketDiscount(){
  return basePrice() * 0.95;
}

double regularDiscount(){
  return basePrice() * 0.98
}

Answer 7

Вам (как оригинальному Аскеру, так и Джеймсу Керрану) не хватает того, что в этом конкретном примере расчет не повторяется. Либо код идет вниз по одной ветви if и вызывает basePrice(), который выполняет вычисление. Или код переходит в другую ветвь if, и там также выполняется вычисление.

Умножение выполняется независимо, но все же только один раз. Так же, как и во временной версии.

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

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

Именно тогда пригодится обратный рефакторинг «Замени запрос на временную температуру».

Answer 8

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