We read the linear code more
You can only consider yourself a rebel when people actually defend the opposite position of yours. I disagree with one of the best practices recently presented on the Google Testing Blog. This is usually a very good resource, because this post didn’t find its way into my newsreader by accident!
The authors presented two versions of the function and asked which one was more readable.
You can immediately notice that the feed is slightly biased. In addition to the obvious color skew, the authors replaced some of the code on the right with dots, making both implementations roughly the same size, even though the right is actually much longer.
The authors claim that the function on the right is more readable because it does not mix levels of abstraction, making its structure more consistent with the top-down principle. OK, let’s say, but the function on the left reads linearly from the top of the screen to the bottom, and if you want to understand what’s going on in the function on the right, you have to jump all over the code.
You may say: yes, that’s fair, but the whole code is never to be learned and not necessary, that’s what abstraction is for! Let’s admit. Yes, now answer the question quickly: does the function that bakes the pizza also preheat the oven, or does it need to be preheated? Hint: there is two functions. One is called bake
she receives at the entrance Pizza, and the second is called bakePizza
…
Another question: what happens if you pass pizza to these functions twice? Are they idempotent or will you end up eating coal?
So, as you might have guessed, I don’t like the design style of the code on the right. But I must admit, for some reason it is easier to understand than the code on the left. Is it because of the structure that separates the levels of abstraction? Let’s see. How about this version?
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
}
Do you know? This is just the code of the function on the left with the function names on the right commented out.
I don’t know about you, but I like him the most. And it seems that the readability has increased only because of the explanation of what we are doing, not because of additional layers of abstraction and indirection.
Finally, I agree with the sentiment above: it’s not a good idea to extract small functions from inline code, especially if you only use them once. No benefits will outweigh the loss of linearity.
And one more
I wasn’t sure if I should talk about it, but I ended up editing the post because I’m concerned about working with the oven in this feature.
First, preheating the oven is an autonomous operation and should probably be done by the oven itself. Moreover, this code is illogical: why create a new pizza oven? In fact, we are buying a stove onceand then we bake all the pizzas in it without repeating the entire heating cycle.
I know this is a synthetic example, but similar problems actually occur in real code, sometimes causing performance issues. This code probably has get furnace as a parameter. Providing it is the task of the insolent party.
(And after we put the pizza in the box, we most likely need an interface to return the box, not the pizza. Oh well.)