5 ms·
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 p
by 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 agoAgree. 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 agoThe 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 agoRe: '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)
- makeitdouble 3y agoI think the lines gets blurred when doing early exits and guard conditions. For instance if !condition_a && !conditon_b { return fn_d() // probably an error condition ? } if condition_a && condition_b { return fn_a() } if condition_a { fn_b() } else { fn_c() }
- 1letterunixname 3y agoThese 3 examples optimize to the the same thing because they generate identical instructions: https://godbolt.org/z/a1Yq9rceE https://godbolt.org/z/a1Yq9rceE Note: Inverting and rearranging conditions changes what LLVM decides to output, sometimes for the worse. --opt-level=s is counterproductive here.
- titzer 3y agoI think I would like the former syntax. Witness one of the several nested matching situations I run into: https://github.com/titzer/wizard-engine/blob/master/src/engine/Type.v3#L272 https://github.com/titzer/wizard-engine/blob/master/src/engi... This would be much, much better if Virgil had pattern matching on tuples of ADTs.
- agumonkey 3y agoI really prefer the flat variant cause it helps me ensure exhaustiveness. In the end people probably trained their brain to read the nested variant to the point it's transparent to their neurons.
- cratermoon 3y agoOh no, hard disagree. The match implementation is far easier to reason about. I can see at a glance that if both condition_a and condition_b are true, call fn_a(). For the nested if version I have to trace each expression by hand.
- Izkata 3y agoThey said "isn't", not "is". Double negative. You two agree.
- whartung 3y ago> saying these are rule of thumbs they're not always taken this way by young programmers. The important thing to learn about all “rules of thumb” and “best practices” is the WHY behind them. Programmers especially should not copy/paste these things and apply them by rote. RoT and BP blindly applied may well not be a good idea. As with everything “it depends”.
- keithnz 3y agogiven it looks state mutating, I'd write this something like walruses.apply(condition ? frobnicate : transmorfify) where apply does the looping.
- tacitusarc 3y agoI think this is actually a great example of why `if` should be "pushed up." The objective of the code is to perform an particular operation on the walrus, given a condition. The is actually ifs being instead of polymorphism and a type system. Why does the walrus have these two functions, which must be called in different scenarios? Why not have a single function, and two types, and the correct type is passed in? Even given the current structure, we could accomplish this: // There are better ways to accomplish this, depending on the language func frobnicate(walrus) { return walrus.frobnicate(); } func transmogrify(walrus) { return walrus.transmogrify(); } // Somewhere high in the code if condition { processing_function = frobnicate } else { processing_function = transmogrify } // Somewhere else in the code for walrus in walruses { processing_function() } If the decisions are made as early as possible, they do not need to be littered throughout the code. The guts of the code can run without branches, performing the same operations every time, the output only modified through the construction graph. Of course, this is not a new idea: https://www.youtube.com/watch?v=4F72VULWFvc https://www.youtube.com/watch?v=4F72VULWFvc It was old 15 years ago.
- hedora 3y agoThat pattern is not zero-cost: It breaks inlining and other compile-time optimizations. It also can hurt maintainability and readability. If the type of processing_function() is significantly generic, figuring out what is being called in the loop can be hard (as in undecidable; it is equivalent to the halting problem). In an extreme case that I've seen in real code, processing_function() is called send(t: Object) -> Object or recv(u: Object), and all non-trivial control flow in the system is laundered through those two function names. So, you have 100's or 1000's of send, recv pairs, and grep (or even a fancy IDE) can't match them up.
- tacitusarc 3y agoFor a non-trivial codebase, this style is incredibly valuable for readability. With it, decisions about execution are made in a single place, rather than distributed throughout the code. If the conditionals are embedded deeply in the call graph, sure a reader may be able to understand a single function when looking at it, but they won't know the implications of that function or be able to reason about the program as a whole. I've seen a huge class of bugs that are eliminated by properly using polymorphism rather than embedded conditionals. That being said, yeah, its possible to just layer unnecessary abstraction and way over complicate things, making the layers utterly incomprehensible. But that seems to be the result of a misunderstanding- the point here isn't to _abstract_, it's to properly model polymorphic behavior. Any abstraction that takes place to do that is suspicious at best. I think the performance aspect is negligible for 98% of programs. The compiler may or may not be able to inline the underlying function, depends on the compiler. Depends on the optimizations. And in almost all cases, if you're relying on compiler optimizations to fix your performance problems, you're looking in the wrong place. Not to say such cases don't exist, and there's a slew of people who work in those areas, but I trust they know their business well enough to evaluate the trade offs here and make the right decisions.
- swsieber 3y ago> But junior programmers or students reading dogmatic-like wise-sounding rules may produce worse code to strictly follow these kind of advices. Yes. I for one am happy such articles exist. Thus one articulated something I've run into quite a few times without being able to fully articulate the issues. It seems like a nice mental model to have in my back pocket. That said, I appreciate comments like your because I hope the dogmatic junior dev comes across it and hopefully becomes a little more nuanced.
- matklad 3y agoThanks! You are exactly the target audience for this article! It really is only useful when: * you already have the intuitive understanding of problems being described * but you haven’t quite yet verbalized the problem and connected the dots This clicked for me yesterday, happy that it helped you to articulate what you’ve already knew today!
- Olreich 3y agoI’m close to giving up on dogmatic junior devs. They don’t ever seem to give up their dogma, even in the face of direct damage caused by their dogma. The closest I’ve seen is a dogmatic junior dev saying they understand and will relax on the dogma, but turning around and enforcing it when they think you aren’t looking. I’ve seen dogmatic junior devs turn into dogmatic senior devs, but I’ve never seen a dogmatic junior dev turn into a pragmatic senior dev.
- tenahu 3y agoThat was my same thought. This example would only work in specific circumstances, so why present it as a rule,