Бунтарём себя можно считать только тогда, когда люди на самом деле защищают противоположную вашей позицию. Я не согласен с одной из best practices, недавно представленной в Google Testing Blog . Обычно это очень хороший ресурс, ведь этот пост не случайно попал в мою читалку новостей!
Авторы представили две версии функции и спросили, какая из них более читаема.
Можно сразу заметить, что подача немного пристрастна. В дополнение к очевидному цветовому перекосу авторы заменили часть кода справа многоточиями, сделав обе реализации примерно одного размера, хотя на самом деле правая гораздо длиннее.
Авторы заявляют, что функция справа более читаема, потому что не смешивает уровни абстракции, благодаря чему её структура больше отвечает принципу «сверху вниз». Хорошо, допустим, но функция слева линейно читается сверху экрана донизу, а если вы захотите разобраться, что происходит в правой функции, то вам придётся скакать по всему коду.
Вы можете сказать: да, справедливо, но весь код целиком никогда изучать и не нужно, именно для этого и нужна абстракция! Допустим. Так, а теперь ответьте быстро на вопрос: выпекающая пиццу функция выполняет ли также и разогрев печи, или её нужно предварительно разогреть? Подсказка: там есть две функции. Одна называется bake, она получает на входе Pizza, а вторая называется bakePizza…
Ещё один вопрос: что произойдёт, если передать пиццу этим функциям дважды? Они идемпотентны или в конечном итоге вам придётся есть угли?
Итак, как вы уже могли догадаться, мне не нравится стиль оформления кода справа. Но должен признать, его почему-то проще понимать, чем код слева. Благодаря ли это структуре, разделяющей уровни абстракций? Давайте посмотрим. Как насчёт такой версии?
func createPizza(order *Order) *Pizza {
// Готовим пиццу
pizza := &Pizza{Base: order.Size,
Sauce: order.Sauce,
Cheese: “Mozzarella”}
// Добавляем топпинги
if order.kind == “Veg” {
pizza.Toppings = vegToppings
} else if order.kind == “Meat” {
pizza.Toppings = meatToppings
}
oven := oven.New()
if oven.Temp != cookingTemp {
// Разогреваем печь
for (oven.Temp < cookingTemp) {
time.Sleep(checkOvenInterval)
oven.Temp = getOvenTemp(oven)
}
}
if !pizza.Baked {
// Печём пиццу
oven.Insert(pizza)
time.Sleep(cookTime)
oven.Remove(pizza)
pizza.Baked = true
}
// Кладём в коробку и нарезаем на куски
box := box.New()
pizza.Boxed = box.PutIn(pizza)
pizza.Sliced = box.SlicePizza(order.Size)
pizza.Ready = box.Close()
return pizza
}Узнаёте? Это всего лишь код функции слева, к которой в качестве комментариев добавлены имена функций справа.
Не знаю, как вам, а мне он нравится больше всего. И похоже, что читаемость повысилась только благодаря объяснению того, что мы делаем, а не из-за дополнительных слоёв абстракции и косвенных действий.
В заключение хочу сказать, что придерживаюсь приведённого выше мнения: не стоит выделять мелкие функции из линейного кода, особенно если вы используете их только один раз. Никакие преимущества не перевесят потерю линейности.
И ещё одно
Я не был уверен, стоит ли об этом говорить, но в результате отредактировал пост, потому что в этой функции меня беспокоит работа с печью.
Во-первых, предварительный разогрев печи - это автономная операция и она, вероятно, должна быть методом самой печи (oven). Более того, этот код нелогичен: зачем создавать совершенно новую печь для изготовления пиццы? В реальности мы покупаем печь один раз, а затем печём в ней все пиццы, не выполняя весь цикл нагрева заново.
Я знаю, что это синтетический пример, но подобные проблемы на самом деле возникают и в реальном коде, иногда вызывая проблемы с производительностью. Вероятно, этот код должен получать печь в качестве параметра. Её предоставление —задача вызывающей стороны.
(И после того, как мы поместили пиццу в коробку, нам, скорее всего, нужен интерфейс для возврата коробки, а не пиццы. Ну да ладно.)
