Seems more applicable to an imperative style, and IMO even still the advice is too dependent on special/actual case details to be generally applicable as a "rule of thumb".
This is just one specific example amongst many of how redundant logic could be simplified.
The first one is a good point in principle, but I would call it pushing down the ifs. A method is usually used to group logic together. So pulling the assertion checks out of that method forces you to replicate it every single time before calling the function. It's very likely you forget to update these conditions in one place and not worth the miniscule loss of performance.
Now, if frobnicate is only something a walrus can do, then it should take a walrus. But if it is something like "given a picture of an animal, check if it is of a walrus, determine its sex and if it is a male, then give the likelihood that is the dominant male on this part of the beach" then you should most certainly push the ifs down to contain all that logic in one single frobnicate method.
Regarding the loops:
If the example given is really all the code you have, then sure, pull that condition out of the loop if you want. But the compiler will optimize it anyway if the condition does not change every loop iteration. And if it does you can't pull the condition out of the loop anyway. A for loop I have much more often in my code looks like this though:
//BAD?
for walrus in walruses {
if !walrus.has_tracker() {
continue;
}
if weather.is_bad() {
continue;
}
let weight = estimate_weight(walrus);
if weight <= threshold_weight {
continue;
}
if study.is_about_frovnication {
walrus.frobnicate()
} else {
walrus.transmogrify()
}
}
Which you do not want to spread to two for loops! Sure, in this toy example the if conditions could be simplified. And if you have such trivial loops, you should use iterators anyway. But sometimes you have logic you need to apply to every element, do not want to put that logic in the frobnicate method (especially if you push your ifs up) and can't use iterators in an elegant way.
For the love of god, do not turn it into
if study.is_about_frobnication {
for walrus in walruses {
if !walrus.has_tracker() {
continue;
}
if weather.is_bad() {
continue;
}
let weight = estimate_weight(walrus);
if weight <= threshold_weight {
continue;
}
walrus.frobnicate()
} else {
for walrus in walruses {
if !walrus.has_tracker() {
continue;
}
if weather.is_bad() {
continue;
}
let weight = estimate_weight(walrus);
if weight <= threshold_weight {
continue;
}
walrus.transmogrify()
}
And last but not least, Profile before you optimize anything. The part about the ifs, and the enum example may not actually impact your performance at all! The compiler is very smart about moving code around, so the enum thing would most likely be inlined and then reduced down further, potentially eliminating the enum from the code completely. It can still be a worthwhile code change to make, because it improves legibility. But since the author mentioned the performance a few times I wanted to give a counterpoint. (That being, profile before you optimize!)
I agree with part of the article, because I didn't read the rest. I truly dislike the use of single letter variable names: f, g, h and foo, bar, baz. My advice: use descriptive variable names.
function twoIfs, function complicatedIf, var simpleAnd, etc. Makes it so much easier to read examples instead of remembering "oh yeah, f had two ifs in it, h had the if/else, g calls f which calls h which,...".
Also see this often in other examples: "A for 'Truthy variable' " 😓 Wtf. Laziness is good when it makes things easier, not harder.
I'm reasonably sure compilers can shift the if out. I believe it's called "loop invariant code motion". Won't work in call cases, but in the variable case it should.
I’m reasonably sure compilers can shift the if out. I believe it’s called “loop invariant code motion”.
That scenario would only apply if the condition was constant and specified at compile time. There's no indication on whether var1 or var2 are compile-time constants or predicates evaluated at runtime.
The variable is set once, but the if expression is still evaluated every time (unless the compiler can optimize it)
(edit after skimming the article: yes,using the variable would solve the problem of the last example)
So there would be the branching overhead in every iteration. But that's something the cpu branch prediction should cover, especially since the taken branch will be identical in every loop.
Same also applied to the implied condition to break the for loop (only the first few and last iteration should be wrong predictions)
Modern optimizing compilers are magical. I would need to check assembly but I would actually expect the if to be hoisted out of the loop entirely to relieve pressure on the branch predictor.
Saw this posted on hackernews yesterday, along with hundreds of comments of people completely misunderstanding the advice given. Glad to not see any of that here.