50
Push Ifs Up And Fors Down (matklad.github.io)
you are viewing a single comment's thread
view the rest of the comments
[-] Turun@feddit.de 12 points 1 year ago

I disagree with this article.

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!)

this post was submitted on 17 Nov 2023
50 points (90.3% liked)

Programming

17314 readers
77 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities !webdev@programming.dev



founded 2 years ago
MODERATORS