Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Push Ifs Up and Fors Down

matklad.github.io

Post reply on HN
Print view View original

41–50 of 209 posts

Re: Push Ifs Up and Fors Down

#41

Post by quantadev »

There's always a trade-off between performance v.s. clearness in the code.

If a certain function has many preconditions it needs to check, before running, but needs to potentially run from various places in the code, then moving the precondition checks outside the method results in faster code but destroys readability and breaks DRY principle.

In cases where this kind of tension (DRY v.s. non-DRY) exists I've sometimes named methods like 'maybeDoThing' (emphasis on 'maybe' prefix) indicating I'm calling the method, but that all the precondition checks are inside the function itself rather than duplicate logic all across the code, everywhere the method 'maybe' needs to run.

Re: Push Ifs Up and Fors Down

#42

Post by magicalhippo »

neRok wrote: post #30

I agree that the first example in the article is "bad"... fn frobnicate(walrus: Option )`) but the rest makes no sense to me! // GOOD frobnicate_batch(walruses) // BAD for walrus in walruses { frobnicate(walrus) } It doesn't follow through with the "GOOD" example though... fn frobnicate_batch(walruses) for walrus in walruses { frobnicate(walrus) } } What did that achieve? And the next example... // GOOD if condition…

> What did that achieve?

An interface where the implementation can later be changed to do something more clever.

At work we have a lot of legacy code written the BAD way, ie the caller loops, which means we have to change dozens of call sites if we want to improve performance, rather than just one implementation.

This makes it significantly more difficult than it could have been.

Re: Push Ifs Up and Fors Down

#43

Post by shawnz »

Sometimes I like to put the conditional logic in the callee because it prevents the caller from doing things in the wrong order by accident.

Like for example, if you want to make an idempotent operation, you might first check if the thing has been done already and if not, then do it.

If you push that conditional out to the caller, now every caller of your function has to individually make sure they call it in the right way to get a guarantee of idempotency and you can't abstract that guarantee for them. How do you deal with that kind of thing when applying this philosophy?

Another example might be if you want to execute a sequence of checks before doing an operation within a database transaction. How do you apply this philosophy while keeping the checks within the transaction boundary?

Re: Push Ifs Up and Fors Down

#44

Post by dataflow »

andyg_blog wrote: post #21

A more general rule is to push ifs close to the source of input: https://gieseanw.wordpress.com/2024/06/24/dont-push-ifs-up-p... It's really about finding the entry points into your program from the outside (including data you fetch from another service), and then massaging in such a way that you make as many guarantees as possible (preferably encoded into your types) by the time it reaches any core logic, especially…

Doesn't this obfuscate what assumptions you can make when trying to understand the core logic? You prefer to examine all the call chains everywhere?

Re: Push Ifs Up and Fors Down

#45

Post by throwawaymaths »

i would agree with the push ifs up except if youre doing options parsing. having a clean line of flow that effectively updates a struct with a bunch of "maybe" functions is much better if youre consistent with it.

anywhere else, push ifs up.

Re: Push Ifs Up and Fors Down

#46

Post by mnahkies »

xp84 wrote: post #33
jt2190 wrote: post #19

Earlier quoted context omitted.

Code scanners reports should be treated with suspicion, not accepted as gospel. Sonar in particular will report “code smells” which aren’t actually bugs. Addressing these “not a bug” issues actually increases the risk of introducing a new error from zero to greater than zero, and can waste developer time addressing actual production issues.

I agree with you. Cyclomatic complexity check may be my least favorite of these rules. I think any senior developer almost always “knows better” than the tool does what is a function of perfectly fine complexity vs too much. But I have to grudgingly grant that they have some use since if the devs in question routinely churn out 100-line functions that do 1,000 things, the CCC will basically coincidentally trigger and…

I wonder if there's any value in these kind of rules for detecting AI slop / "vibe coding" and preempting the need for reviewers to call it out.

Re: Push Ifs Up and Fors Down

#47

Post by avianlyric »

dataflow wrote: post #44
andyg_blog wrote: post #21

A more general rule is to push ifs close to the source of input: https://gieseanw.wordpress.com/2024/06/24/dont-push-ifs-up-p... It's really about finding the entry points into your program from the outside (including data you fetch from another service), and then massaging in such a way that you make as many guarantees as possible (preferably encoded into your types) by the time it reaches any core logic, especially…

Doesn't this obfuscate what assumptions you can make when trying to understand the core logic? You prefer to examine all the call chains everywhere?

This is why we invented type systems. No need to examine call chains, just examine input types. The types will not only tell you what assumptions you can make, but the compiler will even tell you if you make an invalid assumption!

Re: Push Ifs Up and Fors Down

#48

Post by drob518 »

CodesInChaos wrote: post #37
daxfohl wrote: post #10

Earlier quoted context omitted.

Yeah, I immediately thought of "The Wrong Abstraction" by the same author. Putting the branch inside the for loop is an abstraction, saying "the for loop is the rule, and the branch is the behavior". But very often, some new requirement will break that abstraction, so you have to work around it, and the resulting code has an abstraction that only applies in some cases and doesn't in others, or you force a bunch of ex…

I can recommend this article about the "midlayer mistake" https://lwn.net/Articles/336262/

Nice reference.

Re: Push Ifs Up and Fors Down

#50

Post by mnahkies »

jerf wrote: post #40
neRok wrote: post #30

I agree that the first example in the article is "bad"... fn frobnicate(walrus: Option )`) but the rest makes no sense to me! // GOOD frobnicate_batch(walruses) // BAD for walrus in walruses { frobnicate(walrus) } It doesn't follow through with the "GOOD" example though... fn frobnicate_batch(walruses) for walrus in walruses { frobnicate(walrus) } } What did that achieve? And the next example... // GOOD if condition…

I think the "push for loops down" is missing a bit of detail about the why. The author alludes to "superior performance" but I don't think makes it clear how that can happen. Vectorization is a bit obscure and a lot of coders aren't worried about whether their code vectorizes, but there's a much more common example that I have seen shred the performance of a lot of real-world code bases and HTTP APIs, which is functi…

The worst / most amusing example of this I've seen in the wild was a third party line of business application that was sequentially triaging "pending tasks" to assign priority/to workers.

Our cloud provider had an aircon/overheating incident in the region we were using, and after it was resolved network latency between the database and application increased by a few milliseconds. Turns out if you multiply that by a few million/fast arrival rate you get a significant amount of time, and the pending tasks queue backs up causing the high priority tasks to be delayed.

Based on the traces we had it looked like a classic case of "ORM made it easy to do it this way, and it works fine until it doesn't" but was unfortunately out of our control being a third party product.

If they'd fetched/processed batches of tasks from the database instead I'm confident it wouldn't have been an issue.

Post reply on HN