Push Ifs Up and Fors Down

This aligns nicely with how things look in the "small-step" flavour of PL theory / lambda calculus.

In the lingo, expressions are evaluated by repeatedly getting "rewritten", according to rules called reduction rules. e.g., (1 + 2) + 4 might get rewritten to 3 + 4, which would then get rewritten to 7.

There are two sorts of these rules. There are "congruence" rules, which direct where work is to be done ("which subexpression to evaluate next?"); and then there are "computation" rules (as Pierce [1] calls them), which actually rewrite the expression, and thus change the program state.

"Strict"/"non-lazy" languages (virtually every popular general-purpose language? except Haskell) are full of congruence rules – all subexpressions must be fully evaluated before a parent expression can be evaluated. The important exceptions are special constructs like conditionals and indefinite loops.

For conditionals in particular, a computation rule will kick in before congruence rules direct all subexpressions to be evaluated. This prunes the expression tree, now in a very literal sense.

[1]: Benjamin C. Pierce, Types and Programming Languages (recommended!)

My mental model: align with the world the very specific code I'm writing lives in. From domain specifics, to existing patterns in the codebase, to the stage in the data pipeline I'm at, performance profile, etc.

I used to try and form these kinds of rules and heuristics for code constructs, but eventually accepted they're at the wrong level of abstraction to be worth keeping around once you write enough code.

It's telling they tend to resort to made up function names or single letters because at that point you're setting up a bit of a punching bag with an "island of code" where nothing exists outside of it, and almost any rule can make sense.

-

Perfect example is the "redundancies and dead conditions" mentioned: we're making the really convenient assumption that `g` is the only caller of `h` and will forever be the only caller of `h` in order to claim we exposed a dead branch using this rule...

That works on the island, but in an actual codebase there's typically a reason why `g` and `h` weren't collapsed into each other to start.

Can I float an adjacent model? Classes are nouns, functions are verbs.

it's not that weird, this taken to its logical conclusion is effectively prolog's execution model

perfectly good models

That's almost the same thing as parse don't validate: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-va...

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?

> If the function is called from 37 places, should they all repeat the if statement?

the idea here is probably that in this case we might be able to split our function into two implementing true and false branches and then call them from 21 and 16 places respectively

The keyword here is `consider`. The article targets a somewhat specific design problem where this comes up, especially when you use tagged unions or something similar.

If the function is called from 37 places, you need to refactor your code, but to answer your question on that point: it depends. DRY feels like the right answer, but I think we'd have to review an actual code example to decide.

On examples where you're talking about a library function, I think you have to accept that as a library you're in a special place: you're on an ownership boundary. Data is moving across domains. You're moving across bounded contexts, in DDD-speak. So, no, you look after your own stuff.

EnterCriticalSection suggests a code path where strong validation on entry - including if conditions - makes sense, and it should be thought of as a domain boundary.

But when you're writing an application and your regular application functions have if statements, you can safely push them out. And within a library or a critical code section you can move the `if` up into the edges of it safely, and not down in the dregs. Manage your domain, don't make demands of other people's and within that domain move your control flow to the edge. Seems a reasonable piece of advice.

However, as ever, idioms are only that, and need to be evaluated in the real world by people who know what they're doing and who can make sensible decisions about that context.

The way to solve this is to split decisions from execution and that’s a notion I got from our old pal Bertrand Meyer.

    if (weShouldDoThis()) {
        doThis();
    }

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.

IME this is frequently a local optimum though. "Local" meaning, until some requirement changes or edge case is discovered, where some branching needs to happen outside of the loop. Then, if you've got branching both inside and outside the loop, it gets harder to reason about.

It can be case-dependent. Are you reasonably sure that the condition will only ever effect stuff inside the loop? Then sure, go ahead and put it there. If it's not hard to imagine requirements that would also branch outside of the loop, then it may be better to preemptively design it that way. The code may be more verbose, but frequently easier to follow, and hopefully less likely to turn into spaghetti later on.

(This is why I quit writing Haskell; it tends to make you feel like you want to write the most concise, "locally optimum" logic. But that doesn't express the intent behind the logic so much as the logic itself, and can lead to horrible unrolling of stuff when minor requirements change. At least, that was my experience.)

I've always hated code complexity scanners ever since I noticed them complaining about perfectly readable large functions. It's a lot more readable when you have the logic in one place, and you should only be trying to break it up when the details cause you to lose track of the big picture.

There was a thread yesterday about LLMs where somebody asked "what other unreliable tool people accept for coding?"

Well, now I have an answer...

tangent: how did you get that superscript 0 to render in an HN comment?

Maybe write the functions without the checks, then have wrapper functions that just do the checks and then call the internal function?

You’ve kind of answered your own question here.

> 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

In this situation your function is no longer idempotent, so you obviously can’t provide the guarantee. But quite frankly, if you’re having to resort to making individual functions implement state management to provide idempotency, then I suspect you’re doing something very odd, and have way too much logic happening inside a single function.

Idempotent code tends to fall into two distinct camps:

1. Code that’s inherently idempotent because the data model and operations being performed are inherently idempotent. I.e. your either performing stateless operations, or you’re performing “PUT” style operations where in the input data contains all the state the needs to be written.

2. Code that’s performing more complex business operations where you’re creating an idempotent abstraction by either performing rollbacks, or providing some kind of atomic apply abstraction that ensures partial failures don’t result in corrupted state.

For point 1, you shouldn’t be checking for order of operations, because it doesn’t matter. Everything is inherently idempotent, just perform the operations again.

For point 2, there is no simple abstraction you can apply. You need to have something record the desired operation, then ensure it either completes or fails. And once that happens, ensures that completion or failure is persistent permanently. But that kind of logic is not the kind of thing you put into a function and compose with other operations.

I feel like you either flipped the naming or the reasons you cite don't support the conclusion.

Pushing ifs down usually prevents vectorization and the cases article mentions are those non-dry where a similar branch has to be multiplied on a ton of functions down in the stack, often because the type is internally tagged.

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 extra parameters into the abstraction so that it applies everywhere but is hard to follow. Whereas if you hadn't made the abstraction in the first place, the resulting code can be easier to modify and understand.

https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction

> printInvoice(invoice, options)

The function printInvoice should print an invoice. What happens if an invoice cannot be printed due to one of the named conditionals being false? You might throw an exception, or return a sentinel or error type. What do to in that case is not immediately clear.

Especially in languages where exceptions are somewhat frowned upon for general purpose code flow, and monadic errors are not common (say Java or C++), it might be a better option to structure the code similar to the second style. (Except for the portrait format of course, which should be handled by the invoice printer unless it represents some error.)

> while code readability is extremely important, encapsulation is much more important

Encapsulation seems to primarily be a tool for long-term code readability, the ability to refactor and change code locally, and to reason about global behavior by only concerning oneself with local objects. To compare the two metrics and consider one more important appears to me as a form of category error.

> Push everything down for better code readability

> demonstrates arrow anti-pattern

Ewwww gross. No. Do this instead:

if(!printerReady){ return; } if(!printerHasInk){ return; } if(!printerHasPaper){ return; } if(!invoiceFormatIsPortrait){ return; }

Way more readable than an expanding arrow.

> printInvoices(invoices) // much better than

But yes, put the loop into its own function with all of the other assumptions already taken care of? This is good.

Everyone has different opinions as this might be the printer driver on the PC or the printers internal circuit. The printer itself absolutely shouldn't try to spook the wheels when there is no paper. Id stick that check in the function!

> printInvoice(invoice, options) // is much better than > ...

in Elixirland, we'd name that function maybe_print_invoice which I like much better.

I am not sure that the before case maps to the article's premise, and and I think your optimized SIMD version does line up with the recommendations of the article.

For your example loop, the `if` statements are contingent on the data; they can't be pushed up as-is. If your algorithm were something like:

    if (length % 2 == 1) {
      values[i] += 1;
    } else {
      values[i] += 2;
    }

In your optimized SIMD version, you've removed the `if` altogether and are doing branchless computations. This seems very much like the platonic ideal of the article, and I'd expect they'd be a big fan!

My first thought was conditional branches inside the for loop based on the element as well. By any chance, do you know how hard it is for compilers to auto-vectorize something like this? I am generally not sure where the boundary is.