54 ms·

Push ifs up and fors down

sharas- 3y ago
In other words: write small reusable functions. And an orchestrator "script" function to glue them all together. The orchesrator has all the "ifs" what is domain/application specific.
Waterluvian 3y ago
This kind of rule of thumb usually contains some mote of wisdom, but generally just creates the kind of thing I have to de-dogmatize from newer programmers. There’s just always going to be a ton of cases where trying to adhere to this too rigidly is worse. And “just know when not to listen to this advice” is basically the core complexity here.
- actionfromafar 3y ago
  I think this article could be useful as a koan in a larger compilation. Some of the koans should contradict each other.
- gavmor 3y ago
  De-dogmatizing needs to happen, so what? I think these kinds of rules are helpful to play with; adopt them, ride them as far as they go, invert them for a day or year , see where that takes you. You learn their limits, so what? More grist for the palimpsest.
flashback2199 3y ago
Don't compilers, cpu branch prediction, etc all fix the performance issues behind the scenes for the most part?
- malux85 3y ago
  No, compilers (correctly) prefer correctness over speed, so they can optimise “obvious” things, but they cannot account for domain knowledge or inefficiencies further apart, or that “might” alter some global state, so they can only make optimisations where they can be very sure there’s no side effects, because they have to err on the side of caution. They will only give you micro optimisations which could cumulatively speed up sometimes but the burden of wholistic program efficiency is still very much on the programmer. If you’re emptying the swimming pool using only a glass, the compiler will optimise the glass size, and your arm movements, but it won’t optimise “if you’re emptying the correct pool” or “if you should be using a pump instead” - a correct answer to the latter two could be 100,000 times more efficient than the earlier two, which a compiler could answer.
- jchw 3y ago
  The short answer is absolutely not, even when you are sure that it should. Even something as simple as a naive byteswap function might wind up generating surprisingly suboptimal code depending on the compiler. If you really want to be sure, you're just going to have to check. (And if you want to check, a good tool is, of course, Compiler Explorer.)
- Thaxll 3y ago
  Well compilers are good and dumb at the same time.
- bee_rider 3y ago
  Some of the moves seemed to change what an individual function might do. For example they suggested pulling an if from a function to the calling function. Could the compiler figure it out? My gut says maybe; maybe if it started by inlining the callee? But inlining happens based on some heuristics usually, this seems like an unreliable strategy if it would even work at all.
- roywashere 3y ago
  Function call overhead can be a real issue in languages like Python and JavaScript. But you can or should measure when in doubt!
  - hansvm 3y ago
    It's a real issue in most compiled languages too if you're not careful (also a sort of opposite issue; too few functions causing unnecessary bloat and also killing performance).
- ezekiel68 3y ago
  This is just a myth promoted by Big Compiler designed to sell you more compiler.
- Izkata 3y ago
  There's a pretty famous StackOverflow question/answer about branch prediction failure: https://stackoverflow.com/questions/11227809/why-is-processing-a-sorted-array-faster-than-processing-an-unsorted-array https://stackoverflow.com/questions/11227809/why-is-processi...
jmull 3y ago
The rule of thumb is put ifs and fors where they belong -- no higher or lower. And if you're not sure, think about a little more. I don't think these rules are really that useful. I think this is a better variation: as you write ifs, fors and other control flow logic, consider why you're putting it where you are and whether you should move it to a higher or lower level. You want to think about the levels in terms of the responsibility each has. If you can't think of what the demarcations of responsibility are, or they are tangled, then think about it some more and see if you can clarify, simplify, or organize it better. OK, that's not a simple rule of thumb, but at least you'll be writing code with some thought behind it.
- benatkin 3y ago
  If you know where they belong, this post isn't for you.
- crazygringo 3y ago
  Exactly -- write code that matches clear, intuitive, logical, coherent organization. Because easy counterexamples to both of these rules are: 1) I'd much rather have a function check a condition in a single place, than have 20 places in the code which check the same condition before calling it -- the whole point of functions is to encapsulate repeated code to reduce bugs 2) I'd often much rather leave the loop to the calling code rather than put it inside a function, because in different parts of the code I'll want to loop over the items only to a certain point, or show a progress bar, or start from the middle, or whatever Both of the "rules of thumb" in the article seem to be motivated by increasing performance by removing the overhead associated with calling a function. But one of the top "rules of thumb" in coding is to not prematurely optimize. If you need to squeeze every bit of speed out of your code, then these might be good techniques to apply where needed (it especially depends on the language and interpreted vs. compiled). But these are not at all rules of thumb in general.
  - pests 3y ago
    I think a key thing software engineers have to deal with opposed to physical engineers is an ever changing set of requirements. Because of this we optimize for different trade-offs in our codebase. Some projects need it, and you see them dropping down to handwritten SIMD assembly for example. But for the most of us the major concern is making changes, updates, and new features. Being able to come back and make changes again later for those ever changing requirements. A bridge engineer is never going to build abstractions and redundencies on a bridge "just in case gravity changes in the future". They "drop down to assembly" for this and make assumptions that _would_ cause major problems later if things do change (they wont). I guess my point is: optimizing code can mean multiple things. Some people want to carve out of marble - it lasts longer, but is harder to work with. Some people want to carve out of clay - its easier to change, but its not as durable.
nerdponx 3y ago
Pushing "ifs" up has the downside that the preconditions and postconditions are no longer directly visible in the definition of a function, and must then be checked at each call site. In bigger projects with multiple contributors, such functions could end up getting reused outside their intended context. The result is bugs. One solution is some kind of contract framework, but then you end up rewriting the conditions twice, once in the contract and once in the code. The same is true with dependent types. One idea I haven't seen before is the idea of tagging regions of code as being part of some particular context, and defining functions that can only be called from that context. Hypothetically in Python you could write: @requires_context("VALIDATED_XY") def do_something(x, y): ... @contextmanager def validated_xy(x, y): if abs(x) < 1 and abs(y) < 1: with context("VALIDATED_XY"): yield x, y else: raise ValueError("out of bounds") with validated_xy(0.5, 0.5) as x_safe, y_safe: do_something(x_safe, y_safe) # Error! do_something(0.5, 0.5) The language runtime has no knowledge of what the context actually means, but with appropriate tools (and testing), we could design our programs to only establish the desired context when a certain condition is met. You could enforce this at the type level in a language like Haskell using something like the identity monad. But even if it's not enforced at the type level, it could be an interesting way to protect "unsafe" regions of code.
- timeon 3y ago
  > In bigger projects with multiple contributors, such functions could end up getting reused outside their intended context. The result is bugs. Yes but in this particular example `fn frobnicate(walrus: Walrus)` if you pass here anything other then owned Walrus then program would not compile. Even if it was something generic passing the arg would have to satisfy trait bounds. Definition of those bounds in function definition would be required by compiler based on how the argument will be used inside function.
- imron 3y ago
  > Pushing "ifs" up has the downside that the preconditions and postconditions are no longer directly visible in the definition of a function You're missing the second part of the author's argument: "or it could push the task of precondition checking to its caller, and enforce via types" The precondition is therefore still directly visible in the function definition - just as part of the type signature rather than in an if statement. The "enforce preconditions via types" is a common pattern in Rust (the language used in the article), and unlike checking with if statements, it's a strict precondition that is checked at compile time rather than at runtime and you won't even be able to compile your program if you don't meet the pre-condition.
bee_rider 3y ago
It seems like a decent general guideline. It has made me wonder, though—do there exist compilers nowadays that will turn if’s inside inner loops into masked vector instructions somehow?
p4bl0 3y ago
I'm not convinced that such general rules can really apply to real-world code. I often see this kind of rules as ill-placed dogmas, because sadly even if this particular blog post start by saying these are rule of thumbs they're not always taken this way by young programmers. A few weeks ago YouTube was constantly pushing to me a video called "I'm a never-nester" apparently of someone arguing that one should never nest ifs, which is, well, kind of ridiculous. Anyway, back at the specific advice from this post, for example, take this code from the article: // GOOD if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } // BAD for walrus in walruses { if condition { walrus.frobnicate() } else { walrus.transmogrify() } } In most cases where code is written in the "BAD"-labeled way, the `condition` part will depend on `walrus` and thus the `if` cannot actually be pushed up because if it can then it is quite obvious to anyone that you will be re-evaluating the same expression — the condition — over and over in the loop, and programmers have a natural tendency to avoid that. But junior programmers or students reading dogmatic-like wise-sounding rules may produce worse code to strictly follow these kind of advices.
- hollerith 3y ago
  Agree. Also, most of the time, the form that is easier to modify is preferred, and even if `condition` does not currently depend on `walrus`, it is preferable for it to be easy to make it depend on `walrus` in the future.
- gsuuon 3y ago
  The GOOD refactor would only work if the condition didn't depend on `walrus` and helps to make that fact explicit. If you apply "push fors down" again you end up with: if condition { frobnicate_batch(walruses) } else { transmogrify_batch(walruses) }
- ToValueFunfetti 3y ago
  Re: 'never-nesting', I'm not especially dogmatic, but I've never empirically seen a situation where this: match (condition_a, condition_b){ (true, true) => fn_a() (true, false) => fn_b() (false, true) => fn_c() (false, false) => fn_d() } isn't preferable to this: if condition_a { if condition_b { fn_a() } else { fn_b() } else if condition_b { fn_c() } else { fn_d() } (Assuming the syntax is available)
torstenvl 3y ago
I wouldn't quite say this is bad advice, but it isn't necessarily good advice either. I think it's somewhat telling that the chosen language is Rust. The strong type system prevents a lot of defensive programming required in other languages. A C programmer who doesn't check the validity of pointers passed to functions and subsequently causes a NULL dereference is not a C programmer I want on my team. So at least some `if`s should definitely be down (preferably in a way where errors bubble up well). I feel less strongly about `for`s, but the fact that array arguments decay to pointers in C also makes me think that iteration should be up, not down. I can reliably know the length of an array in its originating function, but not in a function to which I pass it as an argument.
- lytigas 3y ago
  > A C programmer who doesn't check the validity of pointers passed to functions and subsequently causes a NULL dereference is not a C programmer I want on my team. I disagree. Interfaces in C need to carefully document their expectations and do exactly that amount of checking, not more. Documentation should replace a strong type system, not runtime checks. Code filled with NULL checks and other defensive maneuvers is far less readable. You could argue for more defensive checking at a library boundary, and this is exactly what the article pushes for: push these checks up. Security-critical code may be different, but in most cases an accidental NULL dereference is fine and will be caught by tests, sanitizers, or fuzzing.
  - jrockway 3y ago
    I agree with that. If a function "can't" be called with a null pointer, but is, that's a very interesting bug that should expose itself as quickly as possible. It is likely hiding a different and more difficult to detect bug. Checking for null in every function is a pattern you get into when the codebase violates so many internal invariants so regularly that it can't function without the null checks. But this is hiding careless design and implementation, which is going to be an even bigger problem to grapple with than random crashes as the codebase evolves. Ultimately, if your problem today is that your program crashes, your problem tomorrow will be that it returns incorrect results. What's easier for your monitoring system to detect, a crashed program, or days of returning the wrong answer 1% of the time? The latter is really scary, depending on the program is supposed to do. Charge the wrong credit card, grant access when something should be private, etc. Those have much worse consequences than downtime. (Of course, crashing on user data is a denial of service attack, so you can't really do either. To really win the programming game, you have to return correct results AND not crash all the time.)
ryanjshaw 3y ago
I wrote some batch (list) oriented code for a static analyzer recently. It was great until I decided to change my AST representation from a tuple+discrimated union to a generic type with a corresponding interface i.e. the interface handled the first member of the tuple (graph data) and the generic type the second member (node data). This solved a bunch of annoying problems with the tuple representation but all list-oriented code broke because the functions operating on a list of generics types couldn't play nice with the functions operating on lists of interfaces. I ended up switching to scalar functions pipelined between list functions because the generic type was more convenient to me than the list-oriented code. The reality is you often need to play with all the options until you find the "right" one for your use case, experience level and style.
- aeonik 3y ago
  I'm curious, why couldn't the list of generic types play nice with functions operating on lists of interfaces?
  - deleted 3y ago
    [deleted]
  - ryanjshaw 3y ago
    Have a look here: https://onecompiler.com/fsharp/3ztmx2uhr https://onecompiler.com/fsharp/3ztmx2uhr Basically we want a "Yes" or a "No" when the family has children: let kidsYN = family |> numberOfChildren |> yesOrNo But we get: error FS0001: Type mismatch. Expecting a INode list but given a Node<Person> list The type 'INode' does not match the type 'Node<Person>' Forcing us to do: let familyAsINode = family |> List.map (fun n -> n :> INode) Sure you can wrap this up in a function but it's ugly and annoying to have to use this everywhere and takes away from your logic. It ends up being better to split your "batch" and "scalar" operations and compose them e.g. by introducing a "mapsum" function: let kidsYN2 = family |> mapsum numberOfChildrenV2 |> yesOrNo
smokel 3y ago
Without a proper context, this is fairly strange, and possibly even bad advice. For loops and if statements are both control flow operations, so some of the arguments in the article make little sense. The strongest argument seems to be about performance, but that should typically be one of the latest concerns, especially for rule-of-thumb advice. Unfortunately, the author has managed to create a catchphrase out of it. Let's hope that doesn't catch on.
- actionfromafar 3y ago
  try let’s hope catch not on
- koonsolo 3y ago
  > Unfortunately, the author has managed to create a catchphrase out of it. Let's hope that doesn't catch on. In you next pull request: "Hey can you push this if up?" :D.
- aktenlage 3y ago
  > The strongest argument seems to be about performance It may be an argument, but it's not a strong one. If the improved code can be written like the author puts it in their example (see below), the condition is constant over the runtime of the loop. So unless you evaluate an expensive condition every time, you are good. Branch prediction will have your back. If condition is just a boolean expression using const values, I'd even guess the compiler will figure it out. if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } Branch prediction should have you covered here. If you can easily rewrite it in
wyager 3y ago
Modern compilers and branch predictors means this doesn't matter 99.9% of the time. If you take the same branch every time 100 times in a row, the processor will optimize the cost of the branch away almost entirely. If the branch condition is not volatile, compilers will usually lift it.
- bhuber 3y ago
  This is mostly true, but sometimes the cost of evaluating the condition itself is non-trivial. For example, if a and b are complex objects, even something as trivial as `if (a.equals(b)) ...` might take a relatively long time if the compiler/runtime can't prove a and b won't be modified between calls. In the worst case, a and b only differ in the last field checked by the equality method, and contain giant collections of some sort that must be iterated recursively to check equality.
  - wyager 3y ago
    "If the branch condition is not volatile, compilers will usually lift it" Usually in any program with well-defined semantics (e.g. not using janky multithreaded mutability), this will be true
- titzer 3y ago
  It's not just the direct cost of a branch, it's the downstream costs as well. Removing (or automatically folding branches in a compiler) can lead to optimizations after the branch. E.g. if (foo > 0) { x = 3; } else { x = 7; } return x * 9; If the compiler (or programmer) knows foo is greater than zero (even if we don't know what it actually is), then the whole thing folds into: return 27; That also means that foo is not even used, so it might get dead-code eliminated. If that gets inlined, then the optimizations just keep stacking up. (not that the article was about that, it's just one implication of removing branches: downstream code can be optimized knowing more). So, in summary, compilers matter.
  - RenThraysk 3y ago
    That code shouldn't generate any branching (unless on an odd cpu). A serious compiler should recognise it can be done with a conditional move.
layer8 3y ago
One variation on this theme is to use subclass or function polymorphism. This lets you decouple (in time) (a) the code that decides what to do based on the condition from (b) the code that actually does what was decided. In TFA’s enum example, instead of the enum values, you could pass the foo/bar functions around as values (or instances of different subclasses implementing a common interface method as either foo or bar), and the place where the operation finally needs to be performed would invoke the passed-around function (or object). I.e., f() would return foo or bar as a function value, and g() would be passed the function value and simply invoke it, instead of doing the `match` case distinction. The drawback is that it’s then less clear in some parts of the code which implementation will be invoked. But the alternative is having to perform the specific operation (directly or indirectly) immediately when the condition is determined. It’s a trade-off that depends on the situation.
jampekka 3y ago
Fors down sounds like a bad advice, and the rationale for it seems to be The Root of all Evil. "Fors up" allows for composition, e.g. map. Fors down makes it clunky at best.
clausecker 3y ago
This reads like the author is very close to rediscovering array oriented programming.
- ezekiel68 3y ago
  In his (for he is a 'he') defense, I believe much of the whole industry has been doing so over the past five years as well. Everything old is new again!
PaulDavisThe1st 3y ago
Contrarily: Push ifs down: BAD: if (ptr) delete ptr; GOOD: delete ptr; Polymorphize your fors: frobnicate (walrus); frobnicate (walruses) { for walrus in walruses frobnicate (walrus); }
- BenFrantzDale 3y ago
  I agrée with them and with you. It looks like they work in some poor language that doesn’t allow overloading. Their example of `frobnicate`ing an optional being bad made me think: why not both? `void frobnicate(Foo&); void frobnicate(std::optional<Foo>& foo) { if (foo.has_value()) { frobnicate(foo); } }`. Now you can frobnicate `Foo`s and optional ones!
  - leduyquang753 3y ago
    Indeed they are working in a poor language that doesn't allow overloading. It's Rust. :-)
myaccountonhn 3y ago
This is a variation of the “golden rule of software quality” https://www.haskellforall.com/2020/07/the-golden-rule-of-software-quality.html?m=1 https://www.haskellforall.com/2020/07/the-golden-rule-of-sof...
deleted 3y ago
[deleted]
andyferris 3y ago
This is kinda "just" predicate push-downs, for imperative code. Makes sense that the author is thinking about it given he is working on databases (tigerbeetle) and part of the motivation is performance. Interesting that we push the ifs up but we push the predicates down! (And a "predicate pushup" sounds like you are adding some randomness to your exercise routine - one, two, skipping this one, four, ...).
1letterunixname 3y ago
condition is an invariant. Unless using cranelift or gcc, it's going to get optimized away by LLVM unless rustc is giving it some non-invariant constraints to solve for. Most compilers, JITs, and interpreters can and do do invariant optimization. Another way to look at the class of problem: if you're using too many conditionals too similarly in many places, you may have created a god type or god function with insufficient abstraction and too much shared state that should be separated. --- Prime directive 0. Write working code. Prime directive 1. Choose appropriate algorithms and data structures suitable for the task being mindful of the approximate big O CPU and memory impact. Prime directive 2. Write maintainable, tested code. This includes being unsurprisingly straightforward. Prime directive 3. Exceed nonfunctional requirements: Write code that is economically viable. If it's too slow, it's unusable. If it's somewhat too slow, it could be very expensive to run or will cost N people M time. Prime directive 4. If it becomes too slow, profile and optimize based on comparing real benchmark data rather than guessing. Prime directive 5. Violate any rule for pragmatic avoidance of absurdity.
assbuttbuttass 3y ago
I love this advice, moving if statements "up" is something I've observed makes a big difference between code that's fun to work with and easy to maintain, and code that quickly gets unmaintainable. I'm sure everyone's familiar with the function that takes N different boolean flags to control different parts of the behavior. I think it really comes down to: functions that have fewer if statements tend to be doing less, and are therefore more reusable.
jackblemming 3y ago
“Put ifs were they minimize the net total Cyclomatic complexity” This is exactly what the factory design pattern is trying to achieve. Figure out the type of object to create and then use it everywhere vs a million different switch statements scattered around. Also don’t create batch functions unless you need to. Functions that work on a single item compose better with map-reduce.
bcrosby95 3y ago
The first example is bad for reasons not related to ifs and fors. In general, if you can, if you have a "container" for something, you should write functions on the contained, domain-level "Thing" rather than the container with the domain level thing. As an example I work with - Clojure. Sometimes I use agents. I don't write functions for agents, I write functions for things agents might contain. Similar rules for Elixir. My primary domain level functions don't work off a PID. They work off the underlying, domain-level data structures. GenServer calls delegate to that where necessary. This makes them more flexible, and tends to keep a cleaner distinction between a core domain (frobnicate the Walrus) and broader application concerns (maybe the Walrus is there, maybe not... oh yeah, also frobnicate it).
- crdrost 3y ago
  Yeah. I think the given advice probably takes validation logic and floats it too high. It is of course nice to have early validation logic, but it is also nice when your functions don't mysteriously crap out with some weird error but instead shout a validation error at you. Haskell solves this with newtypes, “here is a transparent container that certifies that you did the appropriate validation already,” that helps for this. The advice that I really want to hammer into people's heads is, prefer “sad ifs.” That is, I will almost always find this if (something_is_wrong_in_way_1) { // fix it or abort } if (something_is_wrong_in_way_2) { // fix it or abort } if (something_is_wrong_in_way_3) { // fix it or abort } more readable and maintainable than this if (things_are_ok_in_way_1) { if (things_are_ok_in_way_2) { if (things_are_ok_in_way_3) { // do the happy path! } else { // fix or abort thing 3 // if fixed, do the happy path } } else { // fix or abort thing 2 // if fixed, test way 3 again // if way 3 is good do the happy path, else fix it // ... } } else { // ... } I feel like it's in human nature to focus on the expected case, I want everyone whose code I meet to do the exact opposite, focus primarily on the unexpected. Every “if” imposes a mental burden that I am keeping track of, and if you have to go to an external system to fetch that information, or you need to exit early with an error, I can immediately discharge that mental burden the moment I know about it, if the handling and the detection are right next to each other.
deleted 3y ago
[deleted]
chatmasta 3y ago
tl;dr Branch as early as possible (move ifs up) and as infrequently as possible (move loops down, to minimize the number of loops calling something that branches) It probably actually is a good rule of thumb, in that it will naively force you into some local maxima of simplified logic. But eventually it becomes equivalent to saying "all of programming is loops and branches," which is not a very useful statement once you need to decide where to put them...
jasonjmcghee 3y ago
I really thought the whole article was building up to a code example like [fwalrus, twalrus] = split(walrus, condition) frobnicate_batch(fwalrus) transmogrify_batch(twalrus) And instead went for if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } }
- crabmusket 3y ago
  Sort the data! Sort it! [1] [1]: https://macton.smugmug.com/Other/2008-07-15-by-Eye-Fi/n-xmKDH/i-BRtRt6W/A https://macton.smugmug.com/Other/2008-07-15-by-Eye-Fi/n-xmKD...
anyonecancode 3y ago
A good example of this I see a lot in a code base I'm currently working in is React components that conditionally render or not. I really can't stand this pattern, and whenever I can I refactor that into having the component ALWAYS render, but have the caller decide whether or not to call the component.
theteapot 3y ago
Interesting that dependency inversion principal -- [1] is like an extreme case of pushing ifs up by encoding the if into the type interface implemention. Ultimately what you get with DI is pushing an if up some to ... somewhere. # Somewhere: walruses = [new TransWalrus(), new FrobWalarus()], ...] ... for(walrus in walruses) { walrus.transfrobnicaterify() } [1] https://en.wikipedia.org/wiki/Dependency_inversion_principle https://en.wikipedia.org/wiki/Dependency_inversion_principle
- tubthumper8 3y ago
  This only works when `frobnicate` and `transmogrify` have the same argument and return types
bobmaxup 3y ago
As with much programming advice, this is language dependent. You might want branching structures when you have no overloading. You might want guards and other structures when your type checking is dynamic.
norir 3y ago
These heuristics feel to me like a corollary to a simpler, more general rule: avoid redundant work.
gumby 3y ago
The idea of hoisting precondition ifs into the caller is terible! Sure there are special cases where it's a good idea (if nothing else it skips a function call) but in the common case you don't want to this. In a library you want to check preconditions at the external boundary so the actual implementation can proceed knowing there are no dangling pointers, or negative numbers, or whatever the internal assumptions may be. Depending on the caller to do the check defeats the purpose. Also in many cases you would need to violate encapsulation/abstraction. Consider a stupid case: `bool cache_this (T obj)`. Let the cache manager itself check to see if the object is already there as it can probably touch the object fewer times. I agree on the `for` case but it's so trivial the article barely talks about it. Basically it's the same as the encapsulation case above.
- deleted 3y ago
  [deleted]
- LegionMammal978 3y ago
  > In a library you want to check preconditions at the external boundary so the actual implementation can proceed knowing there are no dangling pointers, or negative numbers, or whatever the internal assumptions may be. Depending on the caller to do the check defeats the purpose. I think the idea is to instead address this with a type-safe interface, designed so that the external boundary physically cannot receive invalid input. The caller would then be responsible for its own if statements when constructing the input types from possibly-invalid raw values. > Also in many cases you would need to violate encapsulation/abstraction. Consider a stupid case: `bool cache_this (T obj)`. Let the cache manager itself check to see if the object is already there as it can probably touch the object fewer times. I don't see the suggestion as encouraging such a thing: the "cache_this" check should only ever be performed when it's known for certain that the user wants to access the cached object, so the entry point of the cache abstraction acts as a kind of boundary that the if statement depends on. And the if statement clearly shouldn't be pushed above its own dependency.
  - hedora 3y ago
    In cases where that doesn't make sense: let f = Some(get_a_u16()); foo(f); ... func foo(f: u16) -> u16 { match f { None => 0, Some(f) => f * 1234 } } I'd expect any reasonable compiler to include enough link time optimization and dead code elimination to compile the whole mess down to a single multiply instruction.
stephc_int13 3y ago
A beneficial side effect of this strategy (operating in batches with control logic out of the loop) is that you can also relatively easily distribute the work on many worker threads without touching the interface or the code structure.
wg0 3y ago
This advice falls flat in case of validations. If a function is given some input and is supposed to work with it, how can it avoid if/else and how can we move this logic one level up to the caller to ask the caller to verify every parameter before calling the function? And if we keep pushing (thus pending the decision making) up, wouldn't the top most function become a lot more complicated having a lot more logic pushed up from far down below? That's bad and impractical advice but now will pollute many pull requests with needless arguments.
- LegionMammal978 3y ago
  > If a function is given some input and is supposed to work with it, how can it avoid if/else and how can we move this logic one level up to the caller to ask the caller to verify every parameter before calling the function? The usual way in idiomatic Rust would be to use type safety for this purpose: have the function accept special types for its input, and provide the caller secondary interfaces to construct these types. The constructors would then be responsible for inspecting and rejecting invalid input. This way, the caller can continue pushing the construction, and thus the if/else statements for validation errors, upward to the ultimate source of the possibly-invalid values. (This is also possible in C/C++/Java/C#/..., if not so idiomatic.)
- garethrowlands 3y ago
  No, the advice is good. If the wrong function was called, or a function is called with the wrong input, it’s just a bug and no amount of ‘validations’ can fix it. The article is written in the context of Rust, which has a type system, so the compiler can help check. In cases where a function’s precondition can’t be expressed in the type system, the function should check at the start and bale. For example, it’s reasonable in Java to check if parameters are null (since the compiler cannot do this). For more on this, Google “parse, don’t validate”.
vladf 3y ago
And yet, a Rust Option (or really any option) can just be viewed as a list of one or zero elements. https://rust-unofficial.github.io/patterns/idioms/option-iter.html https://rust-unofficial.github.io/patterns/idioms/option-ite... In fact, in Haskell, operating on an option conditionally has the exact same functor as a list: `map`. So what am I to do, with an iterator? It's conflicting advice! An if is a for for an option.
stevage 3y ago
>// GOOD >if condition { > for walrus in walruses { > walrus.frobnicate() > } >} else { > for walrus in walruses { > walrus.transmogrify() > } >} What? They literally just said in the previous paragraph that the `for` should be pushed down into a batch function.
nialv7 3y ago
I am just really tired of seeing articles like this. Sure, you find some rules that are helpful in some specific cases, but those rules almost never generalize (yeah the irony of me making a generalizing statement here is not lost on me, but I did say "almost"). Imagine you got a corporate programming job, and your manager come to you and says "here, in this company, we keep _all_ the if statements in one function, and no ifs are allowed anywhere else". I would just walk out on the spot. Just stop, stop writing these articles and please stop upvoting them.
- deleted 3y ago
  [deleted]
deleted 3y ago
[deleted]
dg44768 3y ago
Thanks for the article. Maybe I’m confused, but why in the section near the end about how the two recommendations go together, why is the code this: if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } and not this? if condition { frobnicate_batch(walruses) } else { transmogrify_batch(walruses) }
- BenFrantzDale 3y ago
  I thought that too. I think the point there was that you don’t have to push this notion as afar as in/out of functions: that just flipping them within a function can be beneficial.
nurettin 3y ago
I like my ifs down. In fact, after dedades of forcing myself to use parameter checklists and avoiding any nesting, I started to appreciate code that is nested just a couple of times intentionally to get rid of a bunch of conditionals that become implied as a result. It all depends on what feels natural and easy to read at the given stage of your life.
pipeline_peak 3y ago
Please stop using Rust in coding style examples as if it’s something people understand as widely as C. I don’t know your egg head => symbols and idc.
- pipeline_peak 3y ago
  I’m so grateful this wasn’t downvoted/flagged HN has a sense of humor after all :)
ilitirit 3y ago
I'm glad many people have identified why "pushing ifs up" is often bad advice. This article should give examples of when and why you should use either approach. Furthermore, I would argue that there's far too little information and context presented here to even make a decision like that.What do `frobnicate` and `transmogrify` do? How many callers would need to perform these conditional checks? Do these if statements convey domain logic that should actually belong in the walrus class? If these checks need to be made often, would it make better sense to capture the call as a lambda and then only perform the check once instead of having a conditional for loop? Etc etc.
Tainnor 3y ago
The more experience I get, the more I feel that too many programmers worry about making things pretty "in the small", but not enough care about proper design of an entire codebase. I'll admit that I like a concise and well-crafted function just as much as many others, but articles like this one are probably the things that can lead to the kind of unproductive bikeshedding that is sometimes experienced in PRs and other discussions. I don't care that much about whether your function is messy - or about where you put your ifs and fors (or if you use maps and filters instead), as long as the function is properly named, has a good interface (including expressive types), a clear purpose, is properly documented, doesn't make excessive use of side effects etc.
- kubb 3y ago
  Programming language design also tends to focus on the small (e.g. optimizing the number of language keywords).
- tangjurine 3y ago
  Like there's the stuff that can be refactored because the changes are local, or the places to change are clear. And then the stuff that is a lot harder to refactor because you would need to change code in a lot of places, and the places that need to be changed aren't clear. That's probably more the proper design of a codebase stuff.
- Leo_Germond 3y ago
  The advice about if up is not bikeshedding though, it is the exact kind of architectural choice you're saying one should decide on. Don't believe me ? Well imagine you have inputs, where should you validate them ? According to this rule of thumb it's at the topmost level, when they are received. Well that seems super sensible, and it's typically something that helps with understanding the code (rather than checking them at thw very last moment). Also for proofs that's technically necessary to allow the preconditions to "percolate up", which has the same effect of moving the if up. So the first advice is definitely not bike shedding, the second one I'm not so clear though ;)
  - 3y ago
shanghaikid 3y ago
no one likes 'if/else', moving 'if/else' inside/outside a function is not a solution, if you are writing business logic or UI logic, we should try to avoid it as much as possible, only except when you are writing complex algorithm which the computational complexity is required.
crabmusket 3y ago
I was initially surprised by the pushback this article is getting. Then I remembered that this is data-oriented design advice, and I imagine most people on this forum (myself included most of the time) are writing line-of-business web apps where this advice seems like nonsense. I had already internalised the context, and wasn't planning to go apply this to my Laravel backend code. A heuristic: if in your usual daily work you don't need to think about the instruction cache, then you should probably ignore this advice. If you haven't yet and want to get a taste of when this advice matters, go find Mike Acton's "Typical C++ Bullshit" and decipher the cryptic notes. This article is like an understandable distillation of that. Despite what Casey Muratori is trying to argue (and I'm largely sympathetic to his efforts) most line-of-business software needs to optimise for changeability and correctness ("programming over time") not performance.
- GuestHNUser 3y ago
  Yeah, data-oriented design (DOD) always seems to get people riled up. I think it largely gets this type of reaction b/c DOD implies that many aspects of the dominant object-oriented approach are wrong. > most line-of-business software needs to optimise for changeability and correctness, not performance. It's a shame that so many see changeability and performance in opposition with each other. I've yet to find compelling evidence that such is the case.
  - crabmusket 3y ago
    Well it's hard to argue about that tradeoff in general, but I think the existence of languages like Python, Ruby and PHP is compelling. Though I'd accept the argument that they help optimise for neither performance nor changeability! My perspective is necessarily limited, but I often see optimisation as a case of "vertical integration" and changeability as about "horizonal integration". To make something fast, you can dig all the way down through all the layers and do the exact piece of work that is required with the minimum of useless faffing about for the CPU[0]. But to make something robust, you might want to e.g. validate all your inputs at each layer since you don't know who's going to call your method or service. Regarding the DOD/OOP wars, I really love this article, which argues that OOP doesn't have to be bad[1]. I also think that when performance is a requirement, you just have to get more particular about your use of OOP. For example, the difference between Mesh and InstancedMesh[2] in THREE.js. Both are OOP, but have very different performance implications. [0] Casey Muratori's "simple code, high performance" video is an epic example of this. When the work he needed to do was "this many specific floating point operations", it was so cool to see him strip away all the useless layers and do almost exactly and only those ops. [1] https://www.gamedev.net/blogs/entry/2265481-oop-is-dead-long-live-oop/ https://www.gamedev.net/blogs/entry/2265481-oop-is-dead-long... [2] https://threejs.org/docs/index.html?q=instanc#api/en/objects/InstancedMesh https://threejs.org/docs/index.html?q=instanc#api/en/objects...
vjk800 3y ago
I'm surprised by how often programmers coming from software engineering background do this wrong. I started programming in science and there it's absolutely necessary to think about this stuff. Doing for loops in a wrong order can be the difference between running your simulation in one hour instead of one week. With this background, I instinctively do small-time optimization to all my codes by ordering for's and if's appropriately. Code that doesn't do this right just looks wrong to me.
- crabmusket 3y ago
  Watch out for a visit from the premature optimisation police!
  - topaz0 3y ago
    I try to avoid the premature optimisation police just as much as the regular police. Neither of them tend to be serving the public interest, whatever they may say.
cubefox 3y ago
The pushing-ifs-up assumes you are using a language where objects aren't nullable. This doesn't apply to most languages. Otherwise we would just get a potential null pointer exception when we don't check for null inside the function.
theK 3y ago
> If there’s an if condition inside a function, consider if it could be moved to the caller instead Haha, it is important to have logic where it is relevant. If performance is more relevant than semantics or maintainability do that. In all other cases favor locality, filter early and fscking kiss. Why is this news?
blumomo 3y ago
I wrongly assumed this article was about readability and maintainability, two high cost factors if five poorly. But I was wrong, here’s the motivation on “push fors up”: > The primary benefit here is performance. Plenty of performance, in extreme cases.
amelius 3y ago
> While performance is perhaps the primary motivation for the for advice, sometimes it helps with expressiveness as well. Compilers can do this. And I don't think the second part of that sentence is applicable very often.
cbogie 3y ago
i first interpreted this on terms of organization policies, and then clicked the link. functions, they’re just like us.
Vt71fcAqt7 3y ago
The way he explains "pushing `for`s down" is obvious. Restating it would be: only do something you need in a for loop and not something you could do once. But that has nothing to do with what he means by "pushing `if`s up" which is about making `if`s appear es early as possible in the control flow. It doesn't matter if `for`s are early or late in the control flow. What matters is that only things that need to be done n times with n being the loop count of the for loop are in the for loop and everything else is not. And I disagree with "push ifs up" stated unqualified.
nivertech 3y ago
The real answer is "it depends", but in the vast majority of cases it is better to "push down" all control flow structures: both conditional branching and loops. This will make it easier to write the code as a pipeline, and therefore more readable. Some argue that higher-order functions (HoFs) should be "pushed up" even if they make the code more verbose (that's a standard code style in Elixir), while the alternative is more readable - it creates lots of microfunctions, which are also clutter the code. For programming langauges with either succinct control structures (APL family), or with syntactic sugar for them, I see no problem with "pushing them up". Of course there are exceptions, e.g. in case the code needs to be optimized for speed, memory usage, security, or formal code correction.
btbuildem 3y ago
This usually culminates with the patient trying to cram all if/else logic into a type system, which effort eventually leads them to have an aneurysm and they have to be carried away on a stretcher.
sharno 3y ago
What I noticed too for pushing the for loops down is that more functions start taking list of things instead of one thing. This makes it easier down the path when a requirement change and we need to handle multiple things instead of one thing with the same code logic. It decreases overall change in the codebase.
kazinator 3y ago
> The GOOD version is good, because it avoids repeatedly re-evaluating condition, removes a branch from the hot loop, and potentially unlocks vectorization. Compilers can do that: notice that condition is not affected within the loop and hoist it outside. Speaking of which, if the condition is affected by the loop, then the transformation is not correct. E.g. maybe the walruses appear in the list in a specific order, and an earlier walrus can determine a condition affecting the treatment of a later walrus. Don't go into your code randomly swapping for and ifs.
6510 3y ago
In my ignorance I would add that empty functions are really fast. if(condition){carbonate = function(){}} for(dragon in dragons){ carbonate(dragon);/do something else/ }
Snetry 3y ago
I find this to be really interesting advise but I wonder when this is right and when this would be wrong. Especially in C, where a lot of data is passed through pointers, you'd want to make sure the data you are given isn't pointing to nothing and can't always rely on the callee doing it for you.
saghm 3y ago
To me, this rule seems fairly natural, so I'm part of the "surprised at the backlash" camp here for sure. At a high level, "if" is a way of skipping unneeded code, and "for" is a way of repeating code, so why wouldn't you want to skip as much unneeded code as possible and repeat as little code as necessary? This isn't meant to diminish the concerns raised in the comments here, but if the tools we're using to express what we want the computer to do end up working better when we can't use simple heuristics like "doing less stuff to achieve the same results is better than doing more stuff", that seems like a fundamental issue with the tools themselves.

In other words: write small reusable functions. And an orchestrator "script" function to glue them all together. The orchesrator has all the "ifs" what is domain/application specific.

Waterluvian 3y ago

This kind of rule of thumb usually contains some mote of wisdom, but generally just creates the kind of thing I have to de-dogmatize from newer programmers. There’s just always going to be a ton of cases where trying to adhere to this too rigidly is worse. And “just know when not to listen to this advice” is basically the core complexity here.

actionfromafar 3y ago
I think this article could be useful as a koan in a larger compilation. Some of the koans should contradict each other.
gavmor 3y ago
De-dogmatizing needs to happen, so what? I think these kinds of rules are helpful to play with; adopt them, ride them as far as they go, invert them for a day or year , see where that takes you. You learn their limits, so what? More grist for the palimpsest.

flashback2199 3y ago

Don't compilers, cpu branch prediction, etc all fix the performance issues behind the scenes for the most part?

malux85 3y ago
No, compilers (correctly) prefer correctness over speed, so they can optimise “obvious” things, but they cannot account for domain knowledge or inefficiencies further apart, or that “might” alter some global state, so they can only make optimisations where they can be very sure there’s no side effects, because they have to err on the side of caution. They will only give you micro optimisations which could cumulatively speed up sometimes but the burden of wholistic program efficiency is still very much on the programmer. If you’re emptying the swimming pool using only a glass, the compiler will optimise the glass size, and your arm movements, but it won’t optimise “if you’re emptying the correct pool” or “if you should be using a pump instead” - a correct answer to the latter two could be 100,000 times more efficient than the earlier two, which a compiler could answer.
jchw 3y ago
The short answer is absolutely not, even when you are sure that it should. Even something as simple as a naive byteswap function might wind up generating surprisingly suboptimal code depending on the compiler. If you really want to be sure, you're just going to have to check. (And if you want to check, a good tool is, of course, Compiler Explorer.)
Thaxll 3y ago
Well compilers are good and dumb at the same time.
bee_rider 3y ago
Some of the moves seemed to change what an individual function might do. For example they suggested pulling an if from a function to the calling function. Could the compiler figure it out? My gut says maybe; maybe if it started by inlining the callee? But inlining happens based on some heuristics usually, this seems like an unreliable strategy if it would even work at all.
roywashere 3y ago
Function call overhead can be a real issue in languages like Python and JavaScript. But you can or should measure when in doubt!
- hansvm 3y ago
  It's a real issue in most compiled languages too if you're not careful (also a sort of opposite issue; too few functions causing unnecessary bloat and also killing performance).
ezekiel68 3y ago
This is just a myth promoted by Big Compiler designed to sell you more compiler.
Izkata 3y ago
There's a pretty famous StackOverflow question/answer about branch prediction failure: https://stackoverflow.com/questions/11227809/why-is-processing-a-sorted-array-faster-than-processing-an-unsorted-array https://stackoverflow.com/questions/11227809/why-is-processi...

jmull 3y ago

The rule of thumb is put ifs and fors where they belong -- no higher or lower. And if you're not sure, think about a little more. I don't think these rules are really that useful. I think this is a better variation: as you write ifs, fors and other control flow logic, consider why you're putting it where you are and whether you should move it to a higher or lower level. You want to think about the levels in terms of the responsibility each has. If you can't think of what the demarcations of responsibility are, or they are tangled, then think about it some more and see if you can clarify, simplify, or organize it better. OK, that's not a simple rule of thumb, but at least you'll be writing code with some thought behind it.

benatkin 3y ago
If you know where they belong, this post isn't for you.
crazygringo 3y ago
Exactly -- write code that matches clear, intuitive, logical, coherent organization. Because easy counterexamples to both of these rules are: 1) I'd much rather have a function check a condition in a single place, than have 20 places in the code which check the same condition before calling it -- the whole point of functions is to encapsulate repeated code to reduce bugs 2) I'd often much rather leave the loop to the calling code rather than put it inside a function, because in different parts of the code I'll want to loop over the items only to a certain point, or show a progress bar, or start from the middle, or whatever Both of the "rules of thumb" in the article seem to be motivated by increasing performance by removing the overhead associated with calling a function. But one of the top "rules of thumb" in coding is to not prematurely optimize. If you need to squeeze every bit of speed out of your code, then these might be good techniques to apply where needed (it especially depends on the language and interpreted vs. compiled). But these are not at all rules of thumb in general.
- pests 3y ago
  I think a key thing software engineers have to deal with opposed to physical engineers is an ever changing set of requirements. Because of this we optimize for different trade-offs in our codebase. Some projects need it, and you see them dropping down to handwritten SIMD assembly for example. But for the most of us the major concern is making changes, updates, and new features. Being able to come back and make changes again later for those ever changing requirements. A bridge engineer is never going to build abstractions and redundencies on a bridge "just in case gravity changes in the future". They "drop down to assembly" for this and make assumptions that _would_ cause major problems later if things do change (they wont). I guess my point is: optimizing code can mean multiple things. Some people want to carve out of marble - it lasts longer, but is harder to work with. Some people want to carve out of clay - its easier to change, but its not as durable.

nerdponx 3y ago

Pushing "ifs" up has the downside that the preconditions and postconditions are no longer directly visible in the definition of a function, and must then be checked at each call site. In bigger projects with multiple contributors, such functions could end up getting reused outside their intended context. The result is bugs. One solution is some kind of contract framework, but then you end up rewriting the conditions twice, once in the contract and once in the code. The same is true with dependent types. One idea I haven't seen before is the idea of tagging regions of code as being part of some particular context, and defining functions that can only be called from that context. Hypothetically in Python you could write: @requires_context("VALIDATED_XY") def do_something(x, y): ... @contextmanager def validated_xy(x, y): if abs(x) < 1 and abs(y) < 1: with context("VALIDATED_XY"): yield x, y else: raise ValueError("out of bounds") with validated_xy(0.5, 0.5) as x_safe, y_safe: do_something(x_safe, y_safe) # Error! do_something(0.5, 0.5) The language runtime has no knowledge of what the context actually means, but with appropriate tools (and testing), we could design our programs to only establish the desired context when a certain condition is met. You could enforce this at the type level in a language like Haskell using something like the identity monad. But even if it's not enforced at the type level, it could be an interesting way to protect "unsafe" regions of code.

timeon 3y ago
> In bigger projects with multiple contributors, such functions could end up getting reused outside their intended context. The result is bugs. Yes but in this particular example `fn frobnicate(walrus: Walrus)` if you pass here anything other then owned Walrus then program would not compile. Even if it was something generic passing the arg would have to satisfy trait bounds. Definition of those bounds in function definition would be required by compiler based on how the argument will be used inside function.
imron 3y ago
> Pushing "ifs" up has the downside that the preconditions and postconditions are no longer directly visible in the definition of a function You're missing the second part of the author's argument: "or it could push the task of precondition checking to its caller, and enforce via types" The precondition is therefore still directly visible in the function definition - just as part of the type signature rather than in an if statement. The "enforce preconditions via types" is a common pattern in Rust (the language used in the article), and unlike checking with if statements, it's a strict precondition that is checked at compile time rather than at runtime and you won't even be able to compile your program if you don't meet the pre-condition.

bee_rider 3y ago

It seems like a decent general guideline. It has made me wonder, though—do there exist compilers nowadays that will turn if’s inside inner loops into masked vector instructions somehow?

p4bl0 3y ago

I'm not convinced that such general rules can really apply to real-world code. I often see this kind of rules as ill-placed dogmas, because sadly even if this particular blog post start by saying these are rule of thumbs they're not always taken this way by young programmers. A few weeks ago YouTube was constantly pushing to me a video called "I'm a never-nester" apparently of someone arguing that one should never nest ifs, which is, well, kind of ridiculous. Anyway, back at the specific advice from this post, for example, take this code from the article: // GOOD if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } // BAD for walrus in walruses { if condition { walrus.frobnicate() } else { walrus.transmogrify() } } In most cases where code is written in the "BAD"-labeled way, the `condition` part will depend on `walrus` and thus the `if` cannot actually be pushed up because if it can then it is quite obvious to anyone that you will be re-evaluating the same expression — the condition — over and over in the loop, and programmers have a natural tendency to avoid that. But junior programmers or students reading dogmatic-like wise-sounding rules may produce worse code to strictly follow these kind of advices.

hollerith 3y ago
Agree. Also, most of the time, the form that is easier to modify is preferred, and even if `condition` does not currently depend on `walrus`, it is preferable for it to be easy to make it depend on `walrus` in the future.
gsuuon 3y ago
The GOOD refactor would only work if the condition didn't depend on `walrus` and helps to make that fact explicit. If you apply "push fors down" again you end up with: if condition { frobnicate_batch(walruses) } else { transmogrify_batch(walruses) }
ToValueFunfetti 3y ago
Re: 'never-nesting', I'm not especially dogmatic, but I've never empirically seen a situation where this: match (condition_a, condition_b){ (true, true) => fn_a() (true, false) => fn_b() (false, true) => fn_c() (false, false) => fn_d() } isn't preferable to this: if condition_a { if condition_b { fn_a() } else { fn_b() } else if condition_b { fn_c() } else { fn_d() } (Assuming the syntax is available)

torstenvl 3y ago

I wouldn't quite say this is bad advice, but it isn't necessarily good advice either. I think it's somewhat telling that the chosen language is Rust. The strong type system prevents a lot of defensive programming required in other languages. A C programmer who doesn't check the validity of pointers passed to functions and subsequently causes a NULL dereference is not a C programmer I want on my team. So at least some `if`s should definitely be down (preferably in a way where errors bubble up well). I feel less strongly about `for`s, but the fact that array arguments decay to pointers in C also makes me think that iteration should be up, not down. I can reliably know the length of an array in its originating function, but not in a function to which I pass it as an argument.

lytigas 3y ago
> A C programmer who doesn't check the validity of pointers passed to functions and subsequently causes a NULL dereference is not a C programmer I want on my team. I disagree. Interfaces in C need to carefully document their expectations and do exactly that amount of checking, not more. Documentation should replace a strong type system, not runtime checks. Code filled with NULL checks and other defensive maneuvers is far less readable. You could argue for more defensive checking at a library boundary, and this is exactly what the article pushes for: push these checks up. Security-critical code may be different, but in most cases an accidental NULL dereference is fine and will be caught by tests, sanitizers, or fuzzing.
- jrockway 3y ago
  I agree with that. If a function "can't" be called with a null pointer, but is, that's a very interesting bug that should expose itself as quickly as possible. It is likely hiding a different and more difficult to detect bug. Checking for null in every function is a pattern you get into when the codebase violates so many internal invariants so regularly that it can't function without the null checks. But this is hiding careless design and implementation, which is going to be an even bigger problem to grapple with than random crashes as the codebase evolves. Ultimately, if your problem today is that your program crashes, your problem tomorrow will be that it returns incorrect results. What's easier for your monitoring system to detect, a crashed program, or days of returning the wrong answer 1% of the time? The latter is really scary, depending on the program is supposed to do. Charge the wrong credit card, grant access when something should be private, etc. Those have much worse consequences than downtime. (Of course, crashing on user data is a denial of service attack, so you can't really do either. To really win the programming game, you have to return correct results AND not crash all the time.)

ryanjshaw 3y ago

I wrote some batch (list) oriented code for a static analyzer recently. It was great until I decided to change my AST representation from a tuple+discrimated union to a generic type with a corresponding interface i.e. the interface handled the first member of the tuple (graph data) and the generic type the second member (node data). This solved a bunch of annoying problems with the tuple representation but all list-oriented code broke because the functions operating on a list of generics types couldn't play nice with the functions operating on lists of interfaces. I ended up switching to scalar functions pipelined between list functions because the generic type was more convenient to me than the list-oriented code. The reality is you often need to play with all the options until you find the "right" one for your use case, experience level and style.

aeonik 3y ago
I'm curious, why couldn't the list of generic types play nice with functions operating on lists of interfaces?
- deleted 3y ago
  [deleted]
- ryanjshaw 3y ago
  Have a look here: https://onecompiler.com/fsharp/3ztmx2uhr https://onecompiler.com/fsharp/3ztmx2uhr Basically we want a "Yes" or a "No" when the family has children: let kidsYN = family |> numberOfChildren |> yesOrNo But we get: error FS0001: Type mismatch. Expecting a INode list but given a Node<Person> list The type 'INode' does not match the type 'Node<Person>' Forcing us to do: let familyAsINode = family |> List.map (fun n -> n :> INode) Sure you can wrap this up in a function but it's ugly and annoying to have to use this everywhere and takes away from your logic. It ends up being better to split your "batch" and "scalar" operations and compose them e.g. by introducing a "mapsum" function: let kidsYN2 = family |> mapsum numberOfChildrenV2 |> yesOrNo

smokel 3y ago

Without a proper context, this is fairly strange, and possibly even bad advice. For loops and if statements are both control flow operations, so some of the arguments in the article make little sense. The strongest argument seems to be about performance, but that should typically be one of the latest concerns, especially for rule-of-thumb advice. Unfortunately, the author has managed to create a catchphrase out of it. Let's hope that doesn't catch on.

actionfromafar 3y ago
try let’s hope catch not on
koonsolo 3y ago
> Unfortunately, the author has managed to create a catchphrase out of it. Let's hope that doesn't catch on. In you next pull request: "Hey can you push this if up?" :D.
aktenlage 3y ago
> The strongest argument seems to be about performance It may be an argument, but it's not a strong one. If the improved code can be written like the author puts it in their example (see below), the condition is constant over the runtime of the loop. So unless you evaluate an expensive condition every time, you are good. Branch prediction will have your back. If condition is just a boolean expression using const values, I'd even guess the compiler will figure it out. if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } Branch prediction should have you covered here. If you can easily rewrite it in

wyager 3y ago

Modern compilers and branch predictors means this doesn't matter 99.9% of the time. If you take the same branch every time 100 times in a row, the processor will optimize the cost of the branch away almost entirely. If the branch condition is not volatile, compilers will usually lift it.

bhuber 3y ago
This is mostly true, but sometimes the cost of evaluating the condition itself is non-trivial. For example, if a and b are complex objects, even something as trivial as `if (a.equals(b)) ...` might take a relatively long time if the compiler/runtime can't prove a and b won't be modified between calls. In the worst case, a and b only differ in the last field checked by the equality method, and contain giant collections of some sort that must be iterated recursively to check equality.
- wyager 3y ago
  "If the branch condition is not volatile, compilers will usually lift it" Usually in any program with well-defined semantics (e.g. not using janky multithreaded mutability), this will be true
titzer 3y ago
It's not just the direct cost of a branch, it's the downstream costs as well. Removing (or automatically folding branches in a compiler) can lead to optimizations after the branch. E.g. if (foo > 0) { x = 3; } else { x = 7; } return x * 9; If the compiler (or programmer) knows foo is greater than zero (even if we don't know what it actually is), then the whole thing folds into: return 27; That also means that foo is not even used, so it might get dead-code eliminated. If that gets inlined, then the optimizations just keep stacking up. (not that the article was about that, it's just one implication of removing branches: downstream code can be optimized knowing more). So, in summary, compilers matter.
- RenThraysk 3y ago
  That code shouldn't generate any branching (unless on an odd cpu). A serious compiler should recognise it can be done with a conditional move.

layer8 3y ago

One variation on this theme is to use subclass or function polymorphism. This lets you decouple (in time) (a) the code that decides what to do based on the condition from (b) the code that actually does what was decided. In TFA’s enum example, instead of the enum values, you could pass the foo/bar functions around as values (or instances of different subclasses implementing a common interface method as either foo or bar), and the place where the operation finally needs to be performed would invoke the passed-around function (or object). I.e., f() would return foo or bar as a function value, and g() would be passed the function value and simply invoke it, instead of doing the `match` case distinction. The drawback is that it’s then less clear in some parts of the code which implementation will be invoked. But the alternative is having to perform the specific operation (directly or indirectly) immediately when the condition is determined. It’s a trade-off that depends on the situation.

jampekka 3y ago

Fors down sounds like a bad advice, and the rationale for it seems to be The Root of all Evil. "Fors up" allows for composition, e.g. map. Fors down makes it clunky at best.

clausecker 3y ago

This reads like the author is very close to rediscovering array oriented programming.

ezekiel68 3y ago
In his (for he is a 'he') defense, I believe much of the whole industry has been doing so over the past five years as well. Everything old is new again!

PaulDavisThe1st 3y ago

Contrarily: Push ifs down: BAD: if (ptr) delete ptr; GOOD: delete ptr; Polymorphize your fors: frobnicate (walrus); frobnicate (walruses) { for walrus in walruses frobnicate (walrus); }

BenFrantzDale 3y ago
I agrée with them and with you. It looks like they work in some poor language that doesn’t allow overloading. Their example of `frobnicate`ing an optional being bad made me think: why not both? `void frobnicate(Foo&); void frobnicate(std::optional<Foo>& foo) { if (foo.has_value()) { frobnicate(foo); } }`. Now you can frobnicate `Foo`s and optional ones!
- leduyquang753 3y ago
  Indeed they are working in a poor language that doesn't allow overloading. It's Rust. :-)

myaccountonhn 3y ago

This is a variation of the “golden rule of software quality” https://www.haskellforall.com/2020/07/the-golden-rule-of-software-quality.html?m=1 https://www.haskellforall.com/2020/07/the-golden-rule-of-sof...

deleted 3y ago

[deleted]

andyferris 3y ago

This is kinda "just" predicate push-downs, for imperative code. Makes sense that the author is thinking about it given he is working on databases (tigerbeetle) and part of the motivation is performance. Interesting that we push the ifs up but we push the predicates down! (And a "predicate pushup" sounds like you are adding some randomness to your exercise routine - one, two, skipping this one, four, ...).

1letterunixname 3y ago

condition is an invariant. Unless using cranelift or gcc, it's going to get optimized away by LLVM unless rustc is giving it some non-invariant constraints to solve for. Most compilers, JITs, and interpreters can and do do invariant optimization. Another way to look at the class of problem: if you're using too many conditionals too similarly in many places, you may have created a god type or god function with insufficient abstraction and too much shared state that should be separated. --- Prime directive 0. Write working code. Prime directive 1. Choose appropriate algorithms and data structures suitable for the task being mindful of the approximate big O CPU and memory impact. Prime directive 2. Write maintainable, tested code. This includes being unsurprisingly straightforward. Prime directive 3. Exceed nonfunctional requirements: Write code that is economically viable. If it's too slow, it's unusable. If it's somewhat too slow, it could be very expensive to run or will cost N people M time. Prime directive 4. If it becomes too slow, profile and optimize based on comparing real benchmark data rather than guessing. Prime directive 5. Violate any rule for pragmatic avoidance of absurdity.

assbuttbuttass 3y ago

I love this advice, moving if statements "up" is something I've observed makes a big difference between code that's fun to work with and easy to maintain, and code that quickly gets unmaintainable. I'm sure everyone's familiar with the function that takes N different boolean flags to control different parts of the behavior. I think it really comes down to: functions that have fewer if statements tend to be doing less, and are therefore more reusable.

jackblemming 3y ago

“Put ifs were they minimize the net total Cyclomatic complexity” This is exactly what the factory design pattern is trying to achieve. Figure out the type of object to create and then use it everywhere vs a million different switch statements scattered around. Also don’t create batch functions unless you need to. Functions that work on a single item compose better with map-reduce.

bcrosby95 3y ago

The first example is bad for reasons not related to ifs and fors. In general, if you can, if you have a "container" for something, you should write functions on the contained, domain-level "Thing" rather than the container with the domain level thing. As an example I work with - Clojure. Sometimes I use agents. I don't write functions for agents, I write functions for things agents might contain. Similar rules for Elixir. My primary domain level functions don't work off a PID. They work off the underlying, domain-level data structures. GenServer calls delegate to that where necessary. This makes them more flexible, and tends to keep a cleaner distinction between a core domain (frobnicate the Walrus) and broader application concerns (maybe the Walrus is there, maybe not... oh yeah, also frobnicate it).

crdrost 3y ago
Yeah. I think the given advice probably takes validation logic and floats it too high. It is of course nice to have early validation logic, but it is also nice when your functions don't mysteriously crap out with some weird error but instead shout a validation error at you. Haskell solves this with newtypes, “here is a transparent container that certifies that you did the appropriate validation already,” that helps for this. The advice that I really want to hammer into people's heads is, prefer “sad ifs.” That is, I will almost always find this if (something_is_wrong_in_way_1) { // fix it or abort } if (something_is_wrong_in_way_2) { // fix it or abort } if (something_is_wrong_in_way_3) { // fix it or abort } more readable and maintainable than this if (things_are_ok_in_way_1) { if (things_are_ok_in_way_2) { if (things_are_ok_in_way_3) { // do the happy path! } else { // fix or abort thing 3 // if fixed, do the happy path } } else { // fix or abort thing 2 // if fixed, test way 3 again // if way 3 is good do the happy path, else fix it // ... } } else { // ... } I feel like it's in human nature to focus on the expected case, I want everyone whose code I meet to do the exact opposite, focus primarily on the unexpected. Every “if” imposes a mental burden that I am keeping track of, and if you have to go to an external system to fetch that information, or you need to exit early with an error, I can immediately discharge that mental burden the moment I know about it, if the handling and the detection are right next to each other.

deleted 3y ago

[deleted]

chatmasta 3y ago

tl;dr Branch as early as possible (move ifs up) and as infrequently as possible (move loops down, to minimize the number of loops calling something that branches) It probably actually is a good rule of thumb, in that it will naively force you into some local maxima of simplified logic. But eventually it becomes equivalent to saying "all of programming is loops and branches," which is not a very useful statement once you need to decide where to put them...

jasonjmcghee 3y ago

I really thought the whole article was building up to a code example like [fwalrus, twalrus] = split(walrus, condition) frobnicate_batch(fwalrus) transmogrify_batch(twalrus) And instead went for if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } }

crabmusket 3y ago
Sort the data! Sort it! [1] [1]: https://macton.smugmug.com/Other/2008-07-15-by-Eye-Fi/n-xmKDH/i-BRtRt6W/A https://macton.smugmug.com/Other/2008-07-15-by-Eye-Fi/n-xmKD...

anyonecancode 3y ago

A good example of this I see a lot in a code base I'm currently working in is React components that conditionally render or not. I really can't stand this pattern, and whenever I can I refactor that into having the component ALWAYS render, but have the caller decide whether or not to call the component.

theteapot 3y ago

Interesting that dependency inversion principal -- [1] is like an extreme case of pushing ifs up by encoding the if into the type interface implemention. Ultimately what you get with DI is pushing an if up some to ... somewhere. # Somewhere: walruses = [new TransWalrus(), new FrobWalarus()], ...] ... for(walrus in walruses) { walrus.transfrobnicaterify() } [1] https://en.wikipedia.org/wiki/Dependency_inversion_principle https://en.wikipedia.org/wiki/Dependency_inversion_principle

tubthumper8 3y ago
This only works when `frobnicate` and `transmogrify` have the same argument and return types

bobmaxup 3y ago

As with much programming advice, this is language dependent. You might want branching structures when you have no overloading. You might want guards and other structures when your type checking is dynamic.

norir 3y ago

These heuristics feel to me like a corollary to a simpler, more general rule: avoid redundant work.

gumby 3y ago

The idea of hoisting precondition ifs into the caller is terible! Sure there are special cases where it's a good idea (if nothing else it skips a function call) but in the common case you don't want to this. In a library you want to check preconditions at the external boundary so the actual implementation can proceed knowing there are no dangling pointers, or negative numbers, or whatever the internal assumptions may be. Depending on the caller to do the check defeats the purpose. Also in many cases you would need to violate encapsulation/abstraction. Consider a stupid case: `bool cache_this (T obj)`. Let the cache manager itself check to see if the object is already there as it can probably touch the object fewer times. I agree on the `for` case but it's so trivial the article barely talks about it. Basically it's the same as the encapsulation case above.

deleted 3y ago
[deleted]
LegionMammal978 3y ago
> In a library you want to check preconditions at the external boundary so the actual implementation can proceed knowing there are no dangling pointers, or negative numbers, or whatever the internal assumptions may be. Depending on the caller to do the check defeats the purpose. I think the idea is to instead address this with a type-safe interface, designed so that the external boundary physically cannot receive invalid input. The caller would then be responsible for its own if statements when constructing the input types from possibly-invalid raw values. > Also in many cases you would need to violate encapsulation/abstraction. Consider a stupid case: `bool cache_this (T obj)`. Let the cache manager itself check to see if the object is already there as it can probably touch the object fewer times. I don't see the suggestion as encouraging such a thing: the "cache_this" check should only ever be performed when it's known for certain that the user wants to access the cached object, so the entry point of the cache abstraction acts as a kind of boundary that the if statement depends on. And the if statement clearly shouldn't be pushed above its own dependency.
- hedora 3y ago
  In cases where that doesn't make sense: let f = Some(get_a_u16()); foo(f); ... func foo(f: u16) -> u16 { match f { None => 0, Some(f) => f * 1234 } } I'd expect any reasonable compiler to include enough link time optimization and dead code elimination to compile the whole mess down to a single multiply instruction.

stephc_int13 3y ago

A beneficial side effect of this strategy (operating in batches with control logic out of the loop) is that you can also relatively easily distribute the work on many worker threads without touching the interface or the code structure.

wg0 3y ago

This advice falls flat in case of validations. If a function is given some input and is supposed to work with it, how can it avoid if/else and how can we move this logic one level up to the caller to ask the caller to verify every parameter before calling the function? And if we keep pushing (thus pending the decision making) up, wouldn't the top most function become a lot more complicated having a lot more logic pushed up from far down below? That's bad and impractical advice but now will pollute many pull requests with needless arguments.

LegionMammal978 3y ago
> If a function is given some input and is supposed to work with it, how can it avoid if/else and how can we move this logic one level up to the caller to ask the caller to verify every parameter before calling the function? The usual way in idiomatic Rust would be to use type safety for this purpose: have the function accept special types for its input, and provide the caller secondary interfaces to construct these types. The constructors would then be responsible for inspecting and rejecting invalid input. This way, the caller can continue pushing the construction, and thus the if/else statements for validation errors, upward to the ultimate source of the possibly-invalid values. (This is also possible in C/C++/Java/C#/..., if not so idiomatic.)
garethrowlands 3y ago
No, the advice is good. If the wrong function was called, or a function is called with the wrong input, it’s just a bug and no amount of ‘validations’ can fix it. The article is written in the context of Rust, which has a type system, so the compiler can help check. In cases where a function’s precondition can’t be expressed in the type system, the function should check at the start and bale. For example, it’s reasonable in Java to check if parameters are null (since the compiler cannot do this). For more on this, Google “parse, don’t validate”.

vladf 3y ago

And yet, a Rust Option (or really any option) can just be viewed as a list of one or zero elements. https://rust-unofficial.github.io/patterns/idioms/option-iter.html https://rust-unofficial.github.io/patterns/idioms/option-ite... In fact, in Haskell, operating on an option conditionally has the exact same functor as a list: `map`. So what am I to do, with an iterator? It's conflicting advice! An if is a for for an option.

stevage 3y ago

>// GOOD >if condition { > for walrus in walruses { > walrus.frobnicate() > } >} else { > for walrus in walruses { > walrus.transmogrify() > } >} What? They literally just said in the previous paragraph that the `for` should be pushed down into a batch function.

nialv7 3y ago

I am just really tired of seeing articles like this. Sure, you find some rules that are helpful in some specific cases, but those rules almost never generalize (yeah the irony of me making a generalizing statement here is not lost on me, but I did say "almost"). Imagine you got a corporate programming job, and your manager come to you and says "here, in this company, we keep _all_ the if statements in one function, and no ifs are allowed anywhere else". I would just walk out on the spot. Just stop, stop writing these articles and please stop upvoting them.

deleted 3y ago
[deleted]

deleted 3y ago

[deleted]

dg44768 3y ago

Thanks for the article. Maybe I’m confused, but why in the section near the end about how the two recommendations go together, why is the code this: if condition { for walrus in walruses { walrus.frobnicate() } } else { for walrus in walruses { walrus.transmogrify() } } and not this? if condition { frobnicate_batch(walruses) } else { transmogrify_batch(walruses) }

BenFrantzDale 3y ago
I thought that too. I think the point there was that you don’t have to push this notion as afar as in/out of functions: that just flipping them within a function can be beneficial.

nurettin 3y ago

I like my ifs down. In fact, after dedades of forcing myself to use parameter checklists and avoiding any nesting, I started to appreciate code that is nested just a couple of times intentionally to get rid of a bunch of conditionals that become implied as a result. It all depends on what feels natural and easy to read at the given stage of your life.

pipeline_peak 3y ago

Please stop using Rust in coding style examples as if it’s something people understand as widely as C. I don’t know your egg head => symbols and idc.

pipeline_peak 3y ago
I’m so grateful this wasn’t downvoted/flagged HN has a sense of humor after all :)

ilitirit 3y ago

I'm glad many people have identified why "pushing ifs up" is often bad advice. This article should give examples of when and why you should use either approach. Furthermore, I would argue that there's far too little information and context presented here to even make a decision like that.What do `frobnicate` and `transmogrify` do? How many callers would need to perform these conditional checks? Do these if statements convey domain logic that should actually belong in the walrus class? If these checks need to be made often, would it make better sense to capture the call as a lambda and then only perform the check once instead of having a conditional for loop? Etc etc.

Tainnor 3y ago

The more experience I get, the more I feel that too many programmers worry about making things pretty "in the small", but not enough care about proper design of an entire codebase. I'll admit that I like a concise and well-crafted function just as much as many others, but articles like this one are probably the things that can lead to the kind of unproductive bikeshedding that is sometimes experienced in PRs and other discussions. I don't care that much about whether your function is messy - or about where you put your ifs and fors (or if you use maps and filters instead), as long as the function is properly named, has a good interface (including expressive types), a clear purpose, is properly documented, doesn't make excessive use of side effects etc.

kubb 3y ago
Programming language design also tends to focus on the small (e.g. optimizing the number of language keywords).
tangjurine 3y ago
Like there's the stuff that can be refactored because the changes are local, or the places to change are clear. And then the stuff that is a lot harder to refactor because you would need to change code in a lot of places, and the places that need to be changed aren't clear. That's probably more the proper design of a codebase stuff.
Leo_Germond 3y ago
The advice about if up is not bikeshedding though, it is the exact kind of architectural choice you're saying one should decide on. Don't believe me ? Well imagine you have inputs, where should you validate them ? According to this rule of thumb it's at the topmost level, when they are received. Well that seems super sensible, and it's typically something that helps with understanding the code (rather than checking them at thw very last moment). Also for proofs that's technically necessary to allow the preconditions to "percolate up", which has the same effect of moving the if up. So the first advice is definitely not bike shedding, the second one I'm not so clear though ;)
- 3y ago

shanghaikid 3y ago

no one likes 'if/else', moving 'if/else' inside/outside a function is not a solution, if you are writing business logic or UI logic, we should try to avoid it as much as possible, only except when you are writing complex algorithm which the computational complexity is required.

crabmusket 3y ago

I was initially surprised by the pushback this article is getting. Then I remembered that this is data-oriented design advice, and I imagine most people on this forum (myself included most of the time) are writing line-of-business web apps where this advice seems like nonsense. I had already internalised the context, and wasn't planning to go apply this to my Laravel backend code. A heuristic: if in your usual daily work you don't need to think about the instruction cache, then you should probably ignore this advice. If you haven't yet and want to get a taste of when this advice matters, go find Mike Acton's "Typical C++ Bullshit" and decipher the cryptic notes. This article is like an understandable distillation of that. Despite what Casey Muratori is trying to argue (and I'm largely sympathetic to his efforts) most line-of-business software needs to optimise for changeability and correctness ("programming over time") not performance.

GuestHNUser 3y ago
Yeah, data-oriented design (DOD) always seems to get people riled up. I think it largely gets this type of reaction b/c DOD implies that many aspects of the dominant object-oriented approach are wrong. > most line-of-business software needs to optimise for changeability and correctness, not performance. It's a shame that so many see changeability and performance in opposition with each other. I've yet to find compelling evidence that such is the case.
- crabmusket 3y ago
  Well it's hard to argue about that tradeoff in general, but I think the existence of languages like Python, Ruby and PHP is compelling. Though I'd accept the argument that they help optimise for neither performance nor changeability! My perspective is necessarily limited, but I often see optimisation as a case of "vertical integration" and changeability as about "horizonal integration". To make something fast, you can dig all the way down through all the layers and do the exact piece of work that is required with the minimum of useless faffing about for the CPU[0]. But to make something robust, you might want to e.g. validate all your inputs at each layer since you don't know who's going to call your method or service. Regarding the DOD/OOP wars, I really love this article, which argues that OOP doesn't have to be bad[1]. I also think that when performance is a requirement, you just have to get more particular about your use of OOP. For example, the difference between Mesh and InstancedMesh[2] in THREE.js. Both are OOP, but have very different performance implications. [0] Casey Muratori's "simple code, high performance" video is an epic example of this. When the work he needed to do was "this many specific floating point operations", it was so cool to see him strip away all the useless layers and do almost exactly and only those ops. [1] https://www.gamedev.net/blogs/entry/2265481-oop-is-dead-long-live-oop/ https://www.gamedev.net/blogs/entry/2265481-oop-is-dead-long... [2] https://threejs.org/docs/index.html?q=instanc#api/en/objects/InstancedMesh https://threejs.org/docs/index.html?q=instanc#api/en/objects...

vjk800 3y ago

I'm surprised by how often programmers coming from software engineering background do this wrong. I started programming in science and there it's absolutely necessary to think about this stuff. Doing for loops in a wrong order can be the difference between running your simulation in one hour instead of one week. With this background, I instinctively do small-time optimization to all my codes by ordering for's and if's appropriately. Code that doesn't do this right just looks wrong to me.

crabmusket 3y ago
Watch out for a visit from the premature optimisation police!
- topaz0 3y ago
  I try to avoid the premature optimisation police just as much as the regular police. Neither of them tend to be serving the public interest, whatever they may say.

cubefox 3y ago

The pushing-ifs-up assumes you are using a language where objects aren't nullable. This doesn't apply to most languages. Otherwise we would just get a potential null pointer exception when we don't check for null inside the function.

theK 3y ago

> If there’s an if condition inside a function, consider if it could be moved to the caller instead Haha, it is important to have logic where it is relevant. If performance is more relevant than semantics or maintainability do that. In all other cases favor locality, filter early and fscking kiss. Why is this news?

blumomo 3y ago

I wrongly assumed this article was about readability and maintainability, two high cost factors if five poorly. But I was wrong, here’s the motivation on “push fors up”: > The primary benefit here is performance. Plenty of performance, in extreme cases.

amelius 3y ago

> While performance is perhaps the primary motivation for the for advice, sometimes it helps with expressiveness as well. Compilers can do this. And I don't think the second part of that sentence is applicable very often.

cbogie 3y ago

i first interpreted this on terms of organization policies, and then clicked the link. functions, they’re just like us.

Vt71fcAqt7 3y ago

The way he explains "pushing `for`s down" is obvious. Restating it would be: only do something you need in a for loop and not something you could do once. But that has nothing to do with what he means by "pushing `if`s up" which is about making `if`s appear es early as possible in the control flow. It doesn't matter if `for`s are early or late in the control flow. What matters is that only things that need to be done n times with n being the loop count of the for loop are in the for loop and everything else is not. And I disagree with "push ifs up" stated unqualified.

nivertech 3y ago

The real answer is "it depends", but in the vast majority of cases it is better to "push down" all control flow structures: both conditional branching and loops. This will make it easier to write the code as a pipeline, and therefore more readable. Some argue that higher-order functions (HoFs) should be "pushed up" even if they make the code more verbose (that's a standard code style in Elixir), while the alternative is more readable - it creates lots of microfunctions, which are also clutter the code. For programming langauges with either succinct control structures (APL family), or with syntactic sugar for them, I see no problem with "pushing them up". Of course there are exceptions, e.g. in case the code needs to be optimized for speed, memory usage, security, or formal code correction.

btbuildem 3y ago

This usually culminates with the patient trying to cram all if/else logic into a type system, which effort eventually leads them to have an aneurysm and they have to be carried away on a stretcher.

sharno 3y ago

What I noticed too for pushing the for loops down is that more functions start taking list of things instead of one thing. This makes it easier down the path when a requirement change and we need to handle multiple things instead of one thing with the same code logic. It decreases overall change in the codebase.

kazinator 3y ago

> The GOOD version is good, because it avoids repeatedly re-evaluating condition, removes a branch from the hot loop, and potentially unlocks vectorization. Compilers can do that: notice that condition is not affected within the loop and hoist it outside. Speaking of which, if the condition is affected by the loop, then the transformation is not correct. E.g. maybe the walruses appear in the list in a specific order, and an earlier walrus can determine a condition affecting the treatment of a later walrus. Don't go into your code randomly swapping for and ifs.

6510 3y ago

In my ignorance I would add that empty functions are really fast. if(condition){carbonate = function(){}} for(dragon in dragons){ carbonate(dragon);/do something else/ }

Snetry 3y ago

I find this to be really interesting advise but I wonder when this is right and when this would be wrong. Especially in C, where a lot of data is passed through pointers, you'd want to make sure the data you are given isn't pointing to nothing and can't always rely on the callee doing it for you.

saghm 3y ago

To me, this rule seems fairly natural, so I'm part of the "surprised at the backlash" camp here for sure. At a high level, "if" is a way of skipping unneeded code, and "for" is a way of repeating code, so why wouldn't you want to skip as much unneeded code as possible and repeat as little code as necessary? This isn't meant to diminish the concerns raised in the comments here, but if the tools we're using to express what we want the computer to do end up working better when we can't use simple heuristics like "doing less stuff to achieve the same results is better than doing more stuff", that seems like a fundamental issue with the tools themselves.