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.
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.
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.
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.
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.)
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).
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.
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.
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.
I've been impressed watching the crews meticulously replace each cable assembly of the George Washington Bridge over the past year or so. All the work that doesn't require disconnected cables is done in parallel, so you can get a sense of the whole process just driving across once (they've finished the north side so you want to drive into Manhattan for the current view).
It's more or less the same as code migrations we're doing on a regular basis, done far more diligently.
Part of my point though was that the bridge builder of today does not need to take into consideration that the person maintaining it 20 years from now will have to deal with gravity changing. So they can make certain assumptions that will be impossible for future maintainers to ever undo.
Software doesn't have these set-in-stone never-changing requirements. I think we are making similar points.
I think the argument here could be stated sort of as push "type" ifs up, and "state" ifs down. If you're in rust you can do this more by representing state in the type (additionally helping to make incorrect states unrepresentable) and then storing your objects by type.
I have a feeling this guide is written for high performance, while it's true that premature optimization is the devil, I think following this sort of advice can prevent you from suffering a death from a thousand cuts.
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
That's fine, but it's often a good idea to separate "do some work on a thing" and "maybe do work if we have a thing". Using the example in the article, it is sometimes useful to have multiple functions for those cases:
Conditions inside the function are also in line with Postel's law, if we drag it from networking to API design. And in large parts of programming the entire "enforce it with types" (say null check without saying null check) isn't a thing at all. It only gets worse with languages where api evolution and compatibility is done by type-sniffing arguments. Those will just laugh at the idea of pushing an if up.
But it's an interesting discussion nonetheless. What I picked up, even if it wasn't directly mentioned (or I might have missed it?), is that a simple check on the caller side can be nice for the reader. Almost zero cost reading at the call site because the branch is a short one, and chances are the check provides some context that helps to understand what the call is all about:
if(x is Walrus) frobnicate(x);
is not just control flow, it doubles as a friendly reminder that frobnication is that thing you do with Walrusses. So my takeaway is the check stays in the function (I also don't agree with the article), but make it a part of the naming consideration. Perhaps "frobnicateIfWalrus" wouldn't be so bad at all! I already do that occasionally, but perhaps it could happen more often?
This reply is akin to "just think about it and do it perfectly dummy" which might sound "obvious" and "smart" but is really just unhelpful commentary. The ideas provided in this blog are actually a good heuristic for improving code in general, even if they don't apply in all cases. Simple and practical rules of thumb can go a long way for those without enough experience yet to have internalized these kinds of rules.
I don't agree it is unhelpful commentary. I think it makes an important point that just following some rules you found won't be enough because the right answer isn't a result of something arbitrary/universal but is a side effect of a better understanding of the responsibility of each piece of code you're writing.
If by “don’t” you mean “do the opposite” then I agree. The third option is “don’t follow the rule blindly but think about the situation”, and for that case it depends entirely on the person and how well they think.
I do think it is shortsighted, and equally helpful and harmful. Helpful in that it reminds the reader this is only one of many considerations that should be taken into account. Harmful because it borders on implying that this shouldn't even be a consideration taken into account because it is irrelevant.
There isn't an easy shortcut. Experts exist for reasons. Just like Doctors and other specialists, find a good expert you trust and evaluate their evaluation of the topic.
Doctors rely on an enormous number of heuristics and shortcuts exactly like this.
My wife teaches doctors, and so much of what she does is giving them rules of thumb much like this one.
edit: I want to note that I'm pretty ambivalent on the actual advice of the article, just commenting that doctors in my experience have a truly astounding collection of rules of thumb
An expert in a field tends to be an expert of the rules of thumb, the references to consult when the rule of thumb doesn't deliver the desired results, and the nuances and exceptions that are the art part of 'science and useful arts' for their field.
"Keep at it, it can be frustrating at times but if you keep studying and practicing how to become a better developer, you will become one"
There are no shortcuts in life. You need to work at it. Cognizant practice got me there, and I think it will get you there, too.
I get people asking me how to become a programmer all the time -- and I always say the same thing: You can't talk about it, you need to do it. You need to write programs, writing software will make you a better programmer.
Have opinions on how to write good software, be open about these opinions, but also be aware that they could be bad opinions, misinformed, or lacking greater context.
Trying to help too much, and give shortcuts to people who cannot otherwise evaluate when these shouldn't be applied has ill effects.
On the same note, I find the reasonning behind the advice way more interesting than the advice itself. It gives a good framing of the different issues to consider when writing conditional and iterative code.
Basically it should help newcomers to identify the issues they're facing and build their own personal heuristics.
A lot of replies in this tree read to me as "don't concisely express a general heuristic principle, because someone might read it and apply it without nuance".
This way of thinking is infantilizing. It is also self-defeating, because it is itself a failure to apply with nuance a general heuristic principle (don't oversimplify). It's doing what it tells you not to do.
Heuristics are only heuristics, but you have to articulate what they are before you can append "and this is only a heuristic, don't apply it blindly and universally".
Appending "everything depends on context" is the trivial part. The heuristics are the substantial contribution.
I see your side, but take it the other way: the basic message is really "you'll have to think for yourself anyway, heuristics from random strangers won't help", which I don't see as infantilizing.
I see post mortems and issue discussions on public projects as a better resource and contribution than sharing personal heuristics.
Better phrasing might be, this is a really good point, and while you can't rely on it alone, the more of these you learn, the better of a programmer you'll be.
In reality these ‘rules of thumb’ become dogma and people forget why the rules of thumb existed in the first place, or make up other bullshit rules to justify it.
This industry is full of dogma and self-appointed experts and it’s utterly tedious.
> OK, that's not a simple rule of thumb, but at least you'll be writing code with some thought behind it.
This reflects one of my answers to the question: What separates an engineer from a developer?
Engineers are intentional, or at least hope to be as often as possible. On the other hand, developers may arrive at similar or same ends but they're generally more reactive and less intentional.
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.
> 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.
> 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.
Definitely, that's a pattern in many programming languages (including, increasingly, in Python). And in dependently-typed languages, you actually can obtain a proof of validity that gets erased at runtime, leaving nothing but basic unboxed data to be executed.
However that's not always an option in all languages or situations, and it's hard to encode conditions like "must only be called while an event loop is running" that way.
I'm envisioning this partly as kind of a catch-all/fallback mechanism that can either be bolted on a language that doesn't have these kinds of features. But it's not always the case that you can effectively communicate contextual requirements through the types of function parameters, and this would cover that case as well.
If you want to reduce this to anything, it's a workaround for not having monads in your language, with some tradeoffs around composability and dynamic extent.
> "or it could push the task of precondition checking to its caller, and enforce via types"
Or the enforcement could be done via a design-by-contract discipline. Either using classical assertions, or something like reusable specifications in Clojure's spec library.
def do_something(position: ValidatedPosition):
...
position = Position(x, y)
if abs(position.x) > 1 or abs(position.y) > 1:
raise ValueError("out of bounds")
position = ValidatedPosition(position.x, position.y)
do_something(position)
In practice I'd probably have ValidatedPosition encapsulate that validation in it's constructor, but you get the idea. If you attempt to pass a Position to do_something then mypy is going to shout at you for passing an object of the wrong type to it.
Python's type checking definitely isn't as comprehensive as something like Rust, but I have found it increasingly useful for situations like this where you want to ensure the data being passed in has been appropriately handled.
Sure, but now you have to rewrite your code to accept a ValidatedPosition as input and not two floats. In Rust that might be "zero cost", but in Python it certainly is not, even if you implement all this in Cython. And even in the context of a Cython/Rust/whatever extension, what if this is calling a BLAS function or some other external procedure that has specific preconditions?
It's usually fine to use the type state pattern as described by sibling commenters, but sometimes it's expensive or infeasible.
Another situation is less related to performance and more related to managing dynamic state. Consider the following annoyance that arises when we try to attach static types to an old-school Python idiom:
Users will end up with a difficult-to-debug auth failure if they forget to call `login` first. Also, Mypy won't catch this without contorting your code.
We naturally would want to add a test case like this:
def test_error_if_not_logged_in():
app_client = AppClient(url="https://api.example.net/v1")
with pytest.raises(RuntimeError):
app_client._post({})
Thus we'll need to guard every single usage of `self._auth_token` with:
if self._auth_token is None:
raise RuntimeError(...)
And there are three usual approaches for handling this:
1. Disregard the article's advice and push the "if" down, into the _post method, to ensure that we only actually need to write that guard condition once.
2. Write a _check_logged_in method that encapsulates the if/raise logic, use that to at least make it easier to apply the guard where needed.
3. Refactor the internals to use something akin to the type state pattern, which is a funky and cool idea that I support. But it could easily end up turning into an "enterprise fizzbuzz" exercise where your little client library expands into a type-driven onion that obfuscates the business logic.
I'm proposing a 4th way:
4. Declare that _post may only be called from inside a specific dynamic context, and ensure that that dynamic context is only established by login().
Is it better? I'm not sure. But I've never seen it before, and it's kind of appealing as I continue to think about it.
Something I've seen a few times in Go and java: Make login() a constructor for the AppClient, or declare what's essentially a factory called "AppClientConfig" with a login() function to create the AppClient.
I've only recently started to think about using factories like that, so I'm very sure there are patterns of dynamic state I wouldn't have a recipe for, but it's a prety appealing idea to me.
The “push ifs up” advice is not about checking preconditions, it’s about selecting the right code path. If you have a function with a precondition, by all means assert it at the beginning of that function. For example, the Java type system allows nulls, so a function that requires an object should throw an exception if a null is passed.
Each call site is absolutely responsible for ensuring that it only calls a function if its precondition is true. This follows from the definition of precondition. Nothing can change that.
Calling a function in violation of its precondition is a bug (in the caller). And, sure, functions often need code that checks that (to prevent undefined behaviour). But we should distinguish these assertions from our program’s actual control flow. The article is about the latter.
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.
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.
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)
}
I 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.
Oh 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.
> 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”.
I 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.
That 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.
For 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.
If you're using a language with proper static typing and generics support (so C++, Rust, Swift, Go, etc), disabling the optimizer is typically a 10x hit. Most of what the optimizer does relies on statically analyzing call graphs, which it can't do if you're slinging function pointers around all over the place.
Godbolt.org has a great example for Swift. Switch the editor to swift mode, load the "Sum over Array" example, and compile it with defaults. It produces reams and reams of assembly code. Now, pass -O in as a compiler flag to enable optimization, and it produces extremely tight code. (Swift 5.0 even noticed the two functions are semantically identical, and deduped them. 5.9 doesn't, but it also produces more compact code.)
It's a similar story for rust, since the example is written in a functional style with an iterator. The C and C++ versions only have an imperative for loop variant, so the optimizer doesn't matter as much for them. (Though you could easily produce a C++ version of the code that takes a sum() function, and has similar bloat until optimization is enabled.)
> 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.
I’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.
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.
> 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.
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.)
Yeah but, not checking for null in C can cause undefined behavior. One possible outcome of undefined behavior is that your program doesn't even crash, but rather continues running in a weird state. So such a bug doesn't always "expose itself".
If we accept that bugs are inevitable, and that accidentally passing a null pointer to a function is a possible bug, then we also conclude that your code really should include non-null assertions that intentionally abort() the program. (Which run in debug/staging mode but can be disabled in release/production mode.)
Indeed, Rust's own standard library uses this method. There are lots of public-facing unsafe functions that can result in undefined behavior if called incorrectly. But if the standard library is compiled in debug mode (which currently requires the unstable flag -Zbuild-std), then it will activate assertions on many of these unsafe functions, so that they will print a message and abort the program if they detect invalid input.
That raises a more general point. When you can't or don't have compile-time checks, removing run-time checks in production amounts to wearing your seat belt only when driving around a parking lot and then unbuckling when you get on the highway. It's very much the Wrong Thing.
I wouldn't really characterize it that way. You (ideally) shouldn't be hitting code paths in production that you didn't ever hit in testing.
But, in any case, if you are fine with the slight performance hit (though many C/C++ projects are not), you can always just keep assertions enabled in production.
I used to ask this same question in interviews: should C code always check for NULL? My favorite answer was that the code should have a notion of boundaries, runtime checks should happen at the boundaries, and debug-only assertions are nice but not required inside the boundaries.
My rule of thumb is: if the type system doesn't prevent an invalid value, it's your responsibility to prevent it at run-time.
I've been writing a lot of T-SQL lately, which doesn't let you declare a parameter or variable as NOT NULL. So it's a good idea to check for NULLs as early as reasonable - usually at the top of the stored procedure (for parameters). Otherwise, a NULL might propagate unexpectedly deep into the call hierarchy and cause less-than-obvious problems.
Fortunately, the data in the table can be declared as NOT NULL, so these kinds of bugs will usually not corrupt the data, but catching them as early as possible makes life easier. However, if there is piece of logic that writes something to the database depending on the value of some parameter, and that parameter is unexpectedly NULL, that might lead to a wrong thing being written, or a necessary thing not being written at all, effectively corrupting the data.
In C a NULL is often a valid non-dereferencable pointer value. If you're checking for invalid pointer values you need to check for any of the 0xfffffffffffffffe possible invalid values. If all you're doing is checking for NULL you're not checking for invalid values at all.
If the precondition of your function is "parameter p can not be NULL" that's fine, go ahead and check for it. If the precondition is "parameter p must be a valid pointer" well then, good luck to you finding the appropriate assertion condition.
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.
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
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.
> 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
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.
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).
A conditional move is not guaranteed to be faster than a predictable branch. It varies a lot by microarchitecture. It was just an example. Imagine if you had a store in the else branch. Now optimizing away a store means that load elimination may kick in. Folding branches in the compiler unlocks tons of downstream benefits.
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.
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!
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!
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, ...).
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.
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.
“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.
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).
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.
Even better, if you're using a language with pattern matching (rust, any ML, haskell), just put it in a single match with the happy case on top, and the error cases below.
To do this, you need to encode the distinction between happy and sad path in your types. That's good for all sorts of other reasons, so it's not really a drawback.
Personally I only either use if statements that wrap a single line of logic, or a switch/case statement where each branch is a single line of logic.
Most situations are handled by guard clauses, exhaustive type switching, or switching on logical cases. Very occasionally do I have to break this practice for a legitimate reason, but this keeps functions simple to read and to reason about.
I occasionally am unable to avoid an else, but I will always avoid else if. It puts too much reliance on remembering how this condition interacts with the one on the if statement above it, which can often be many lines away. I’d infinitely rather use a case statement where all the alternatives are in an easily-understood table with every condition aligned on successive lines.
Because otherwise you can end up with lots of nested conditions which are very confusing to follow, even though they boil down to "do the thing if these are true, otherwise don't do the thing".
I usually move this up in an ETL process into a function I usually call "Pre_Flight_Checklist." Over the years this has gotten its own helper functions. For example, if I am going to use a file, I have function that not only checks for the existence of the file, but that it is a file. This function can also be fed an expected size range and date if "freshness" is expected. If something is out of whack, an error message will mention that a file was expected at /Adirectory/Bdirectory/Cdirectory/filename.ext, but only /Adirectory/Bdirectory was found, nothing under it. I believe in overly-explanatory error messages.
Now, I do run the risk of having things change out from under me, but putting all of the defensiveness up front allows the happy path and some alternatives to be more succinct in the code.
I keep finding new things to be paranoid about, though!
I at first had the opposite reaction, which is that I spend a ton of time fighting inverted ifs! But my reaction is not incompatible with yours. It's not, but it is related: I HATE reading:
if (!someCondition) {
// short amount of code
} else {
// longer happy path
}
The contextual overhead of having to invert a negative (i.e. !!someCondition) is just annoying.
I do agree that if (happy) { /* tons of code / } else { / I forgot how we got here */ } can also be an issue.
The key to the idea above is that you don't have any "else" clause. It's an early return, raise an exception, or modify the data so it's no longer bad.
Yes, that's why I said I realized they weren't quite the same situation, while adding another example of a situation where people create hard to follow structures.
I once had a CS teacher who hated early returns and break statements; they would always prefer the "nested ifs" approach. And they're far from being alone - many others believe that path is cleaner.
I'm with the parent poster, though: if you can use control flow to keep the happy path the unindented path, and you have syntax highlighting to help you show that each guard clause actually does interrupt the control flow, it can often be significantly cleaner.
I also like to think of it in terms of "railway oriented programming" (https://fsharpforfunandprofit.com/rop/ which is a must-read for anyone trying to wade into monads and functional programming IMO) - the "happy path" should be the path that it's easiest for the train to go down, namely a straight line down the page. Only give yourself the headache of a sharp turn when you're interrupting that path!
I don't mind early returns, but I'm infamously anti "break" and "continue" statements (and all my colleagues to date have disagreed with me and have found my stance hilarious and/or infuriating). My rationale is that they turn a loop into a mutant abomination that's a loop and conditionals all mushed up together. I also can't shake this gut feeling that they're crude and antiquated control flow mechanisms, only one step away from the fiery inferno of "goto". So instead of this:
for thing in things:
if thing.is_yellow():
continue
if thing.is_rainbow():
break
thing.thingify()
I'll do:
things_iter = iter(things)
thing = next(things_iter, None)
is_rainbow_thing_found = (
thing.is_rainbow()
if thing is not None
else False
)
while (
thing is not None
and not is_rainbow_thing_found
):
if not thing.is_yellow():
thing.thingify()
thing = next(things_iter, None)
is_rainbow_thing_found = (
thing.is_rainbow()
if thing is not None
else False
)
I’m with your colleagues I’m afraid. The intent of the all-mushed-up-together version is really clear. I had to spend a long time looking at the longer version to figure out what was going on. I know which I’d prefer to maintain if I had to.
Considering break and continue to be too powerful as control flow statements is a somewhat common opinion, in my experience. (I still use them, but only sparingly, and where I am quite sure that they're not confusing.)
That said, your second code block is unreadable to me -- even knowing what it does, it's just confusing. No way that that's more readable than making use of break here. If you don't want so many conditionals in the loop body, you can factor the first check out:
no_yellow_things = (t for t in things if not t.is_yellow())
for t in no_yellow_things:
if t.is_rainbow():
break
t.thingify()
I almost never say this: you are wrong. Code needs to be maintained. Everyone can understand the first example. Everyone has to slowdown and reason about your second example. It is objectively worse. And your argument is flawed: break and continue are not the goto-like behavior that goto is criticized for as they are limited to the scope of that block of code.
The problem traditionally cited with goto is it can go anywhere (even into the body of another function where expectations on which variables are set may not hold).
If you don't like break and continue, how do you justify exceptions? The handling code for those may be in a different file or nowhere. Much closer to goto.
As others have said here, if you want to do it nicely and avoid break / continue, then do it in a proper FP way with chained limit / filter calls, rather than doing it per my example.
I accept your criticism, that my example is much harder to understand, and I admit that it's probably not the way to go in terms of maintainability. However, I'd still argue that my example is closer to an FP approach (even though it still uses a procedural loop), and that it communicates the logic in the same way that an FP approach does. In my example, it's clear that we're filtering out yellow (whereas with "continue", the logic is written in an inverted way); and it's clear just from reading the loop's header that we stop looping when rainbow is found (whereas with "break", logic to stop looping is being added at any arbitrary spot inside the loop's body).
Re: exceptions. I know that many more people object to them, than to break / continue (and whole languages make a point of not using them, most notably Golang). The pros and cons of exceptions is a whole topic in itself that I'd rather not digress into right here. Anyway, personally I'm ok with them. Firstly, they're for errors, and I feel that having weird control flow is sort-of justified for errors, whereas it's not justified for expected behaviour. And secondly, they're at least consistent in that they can occur anywhere, whereas break / continue can only occur inside loops (in Python, at least - in other C-family languages, "break" has different behaviour in different contexts, which is IMHO even worse).
That's the nicest counter-example to my example, thanks for that! I wasn't familiar with take_while() (looks like that's Rust, and looks like Python has a similar itertools.takewhile() ), TIL a neat FP version of break. My example was quite trivial, it's not always so obvious how to break up a convoluted loop, but it should always be possible.
I value being able to grasp what the code does at a glance while scrolling through. Your first snippet is entering my brain like as if without even reading at all, all at once and instantly.
Your second example absolutely does not and I'm sure you know why.
Surely you are joking? Your version is four times longer and repeats a large section of itself.
The way to avoid control flow here is to simply find the rainbow first, take the sequence up until then, filter by yellow, and thingify the remaining things.
rainbow_idx = next(i for i, t in enumerate(things) if t.is_rbow())
yellows = (t for t in things[:rainbow_idx] if t.is_ylw())
for t in yellows:
t.thngry()
Excuse formatting, cellphone.
Not sure this is any better than the obvious solution either.
Yes, I agree, that's the way to do it. Although it isn't always so obvious how to break it up into limit / filter steps, my example was quite trivial. But there should always be a way somehow.
Do they come from the times when structured programming was hot and new, SESE had been the buzzword of the year, and that doodad called "transistor" just had been invented?
I recall avoiding multiple method exits (in Java) like the plague. They were hell to get through in a debugging session. But I guess we don't use debuggers much any more.
When I began writing C# I recall my other team members "refactoring" my code to be shot through with multiple exits instead of a my collecting variable, because "it looked nicer." When they asked me why I wrote it the way I did, I mentioned the Java debugger thing, and they kind of looked at me blankly and said, "Huh, never thought of that."
Why is multiple method exits hell to get through with a debugger? I use a debugger for my C++ code all the time and I’ve never run into an issue because of multiple returns.
Setting breakpoints to inspect local state means you have to catch every single exit. Back then, if you had a thirty second to one minute round trip to get back to the method you were inspecting, it became... frustrating. You probably didn't have to deal with those kind of start-up round trips in C++. With C++ you'd pay on the compile time.
If you wrote the code to have a single exit, the cognitive load was also typically lower, as the control flow could be thought of as one way in, and two ways out (return or exception). Good disciple also followed "the jolly good idea of Demeter" or whatever it came to be called; that being to only operate on the state (or preferably, values) passed in, where possible.
I know this is a late response, but this still doesn’t really make sense to me. What stopped you from placing the break point at the start of the function, before the return values were hit and stepping through the code?
In my experience, having multiple exits is even better because I can check which edge case is being hit by placing multiple breakpoints, or a breakpoint at a specific exit I want to check. I understand the pain of having to step through the code to get back to a certain execution point, but I’ve never been in a situation where multiple exits caused such cognitive load as to hinder that process further.
Correct me if I am misunderstanding, you are saying the first example would be better if it was `walrus.frobnicate()`? Isn't that a syntax preference more than an issue with the point the author is trying to make?
(def walrus (agent {}))
; good
(defn frobnicate [w]
(assoc w :frobnicated true))
(send walrus frobnicate)
; bad
(defn frobnicate [w]
(send walrus #(assoc % :frobnicated true)))
(frobnicate walrus)
It might make sense to also have a function that executes something like (send walrus frobnicate). But IMHO code should basically never look like the bad example.
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...
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.
307 comments
[ 3.2 ms ] story [ 517 ms ] threadThere’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.
Some of the koans should contradict each other.
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.
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.
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.
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.
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.
It's more or less the same as code migrations we're doing on a regular basis, done far more diligently.
When stuck between longevity v less durable, maintainability should be the deciding factor.
Software doesn't have these set-in-stone never-changing requirements. I think we are making similar points.
I have a feeling this guide is written for high performance, while it's true that premature optimization is the devil, I think following this sort of advice can prevent you from suffering a death from a thousand cuts.
That's fine, but it's often a good idea to separate "do some work on a thing" and "maybe do work if we have a thing". Using the example in the article, it is sometimes useful to have multiple functions for those cases:
But also… in languages like Rust, most of the time that second one isn't needed because of things like Option::map.But it's an interesting discussion nonetheless. What I picked up, even if it wasn't directly mentioned (or I might have missed it?), is that a simple check on the caller side can be nice for the reader. Almost zero cost reading at the call site because the branch is a short one, and chances are the check provides some context that helps to understand what the call is all about:
is not just control flow, it doubles as a friendly reminder that frobnication is that thing you do with Walrusses. So my takeaway is the check stays in the function (I also don't agree with the article), but make it a part of the naming consideration. Perhaps "frobnicateIfWalrus" wouldn't be so bad at all! I already do that occasionally, but perhaps it could happen more often?It’s not always helpful to give people the option to do it as they think is best.
If by “don’t” you mean “do the opposite” then I agree. The third option is “don’t follow the rule blindly but think about the situation”, and for that case it depends entirely on the person and how well they think.
1: https://knowyourmeme.com/memes/how-to-draw-an-owl
My wife teaches doctors, and so much of what she does is giving them rules of thumb much like this one.
edit: I want to note that I'm pretty ambivalent on the actual advice of the article, just commenting that doctors in my experience have a truly astounding collection of rules of thumb
There are no shortcuts in life. You need to work at it. Cognizant practice got me there, and I think it will get you there, too.
I get people asking me how to become a programmer all the time -- and I always say the same thing: You can't talk about it, you need to do it. You need to write programs, writing software will make you a better programmer.
Have opinions on how to write good software, be open about these opinions, but also be aware that they could be bad opinions, misinformed, or lacking greater context.
On the same note, I find the reasonning behind the advice way more interesting than the advice itself. It gives a good framing of the different issues to consider when writing conditional and iterative code.
Basically it should help newcomers to identify the issues they're facing and build their own personal heuristics.
This way of thinking is infantilizing. It is also self-defeating, because it is itself a failure to apply with nuance a general heuristic principle (don't oversimplify). It's doing what it tells you not to do.
Heuristics are only heuristics, but you have to articulate what they are before you can append "and this is only a heuristic, don't apply it blindly and universally".
Appending "everything depends on context" is the trivial part. The heuristics are the substantial contribution.
I see post mortems and issue discussions on public projects as a better resource and contribution than sharing personal heuristics.
This industry is full of dogma and self-appointed experts and it’s utterly tedious.
This reflects one of my answers to the question: What separates an engineer from a developer?
Engineers are intentional, or at least hope to be as often as possible. On the other hand, developers may arrive at similar or same ends but they're generally more reactive and less intentional.
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:
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.
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.
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.
However that's not always an option in all languages or situations, and it's hard to encode conditions like "must only be called while an event loop is running" that way.
I'm envisioning this partly as kind of a catch-all/fallback mechanism that can either be bolted on a language that doesn't have these kinds of features. But it's not always the case that you can effectively communicate contextual requirements through the types of function parameters, and this would cover that case as well.
If you want to reduce this to anything, it's a workaround for not having monads in your language, with some tradeoffs around composability and dynamic extent.
Called the Type State Pattern. Here are two links from my bookmarks:
https://cliffle.com/blog/rust-typestate/
https://yoric.github.io/post/rust-typestate/
Or the enforcement could be done via a design-by-contract discipline. Either using classical assertions, or something like reusable specifications in Clojure's spec library.
Or maybe we need more specific semantics around this that can cross cut the domain that public and private (and protected in .NET ecosystems) cover?
Python's type checking definitely isn't as comprehensive as something like Rust, but I have found it increasingly useful for situations like this where you want to ensure the data being passed in has been appropriately handled.
It's usually fine to use the type state pattern as described by sibling commenters, but sometimes it's expensive or infeasible.
Another situation is less related to performance and more related to managing dynamic state. Consider the following annoyance that arises when we try to attach static types to an old-school Python idiom:
Users will end up with a difficult-to-debug auth failure if they forget to call `login` first. Also, Mypy won't catch this without contorting your code.We naturally would want to add a test case like this:
Thus we'll need to guard every single usage of `self._auth_token` with: And there are three usual approaches for handling this:1. Disregard the article's advice and push the "if" down, into the _post method, to ensure that we only actually need to write that guard condition once.
2. Write a _check_logged_in method that encapsulates the if/raise logic, use that to at least make it easier to apply the guard where needed.
3. Refactor the internals to use something akin to the type state pattern, which is a funky and cool idea that I support. But it could easily end up turning into an "enterprise fizzbuzz" exercise where your little client library expands into a type-driven onion that obfuscates the business logic.
I'm proposing a 4th way:
4. Declare that _post may only be called from inside a specific dynamic context, and ensure that that dynamic context is only established by login().
Is it better? I'm not sure. But I've never seen it before, and it's kind of appealing as I continue to think about it.
I've only recently started to think about using factories like that, so I'm very sure there are patterns of dynamic state I wouldn't have a recipe for, but it's a prety appealing idea to me.
https://en.m.wikipedia.org/wiki/Resource_acquisition_is_init...
``` def login(self) -> AppClient ```
Each call site is absolutely responsible for ensuring that it only calls a function if its precondition is true. This follows from the definition of precondition. Nothing can change that.
Calling a function in violation of its precondition is a bug (in the caller). And, sure, functions often need code that checks that (to prevent undefined behaviour). But we should distinguish these assertions from our program’s actual control flow. The article is about the latter.
It has made me wonder, though—do there exist compilers nowadays that will turn if’s inside inner loops into masked vector instructions somehow?
For instance
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.
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.
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”.
Of course, this is not a new idea: https://www.youtube.com/watch?v=4F72VULWFvc
It was old 15 years ago.
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.
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.
Godbolt.org has a great example for Swift. Switch the editor to swift mode, load the "Sum over Array" example, and compile it with defaults. It produces reams and reams of assembly code. Now, pass -O in as a compiler flag to enable optimization, and it produces extremely tight code. (Swift 5.0 even noticed the two functions are semantically identical, and deduped them. 5.9 doesn't, but it also produces more compact code.)
It's a similar story for rust, since the example is written in a functional style with an iterator. The C and C++ versions only have an imperative for loop variant, so the optimizer doesn't matter as much for them. (Though you could easily produce a C++ version of the code that takes a sum() function, and has similar bloat until optimization is enabled.)
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.
* 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!
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.
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.
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.
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.)
If we accept that bugs are inevitable, and that accidentally passing a null pointer to a function is a possible bug, then we also conclude that your code really should include non-null assertions that intentionally abort() the program. (Which run in debug/staging mode but can be disabled in release/production mode.)
But, in any case, if you are fine with the slight performance hit (though many C/C++ projects are not), you can always just keep assertions enabled in production.
(custom assert_always macro, so it doesn't get compiled out in release builds)
I've been writing a lot of T-SQL lately, which doesn't let you declare a parameter or variable as NOT NULL. So it's a good idea to check for NULLs as early as reasonable - usually at the top of the stored procedure (for parameters). Otherwise, a NULL might propagate unexpectedly deep into the call hierarchy and cause less-than-obvious problems.
Fortunately, the data in the table can be declared as NOT NULL, so these kinds of bugs will usually not corrupt the data, but catching them as early as possible makes life easier. However, if there is piece of logic that writes something to the database depending on the value of some parameter, and that parameter is unexpectedly NULL, that might lead to a wrong thing being written, or a necessary thing not being written at all, effectively corrupting the data.
So, defensive programming all the way, baby!
If the precondition of your function is "parameter p can not be NULL" that's fine, go ahead and check for it. If the precondition is "parameter p must be a valid pointer" well then, good luck to you finding the appropriate assertion condition.
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.
Basically we want a "Yes" or a "No" when the family has children:
But we get: Forcing us to do: 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: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.
In you next pull request: "Hey can you push this if up?" :D.
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.
Branch prediction should have you covered here. If you can easily rewrite it inIf 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.
Usually in any program with well-defined semantics (e.g. not using janky multithreaded mutability), this will be true
E.g.
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: 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.
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.
"Fors up" allows for composition, e.g. map. Fors down makes it clunky at best.
Push ifs down:
Polymorphize your fors:https://www.haskellforall.com/2020/07/the-golden-rule-of-sof...
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, ...).
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.
I think it really comes down to: functions that have fewer if statements tend to be doing less, and are therefore more reusable.
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.
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).
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
more readable and maintainable than this 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.Both what you mention, and also the case with :
Which can becomeTo do this, you need to encode the distinction between happy and sad path in your types. That's good for all sorts of other reasons, so it's not really a drawback.
Most situations are handled by guard clauses, exhaustive type switching, or switching on logical cases. Very occasionally do I have to break this practice for a legitimate reason, but this keeps functions simple to read and to reason about.
I occasionally am unable to avoid an else, but I will always avoid else if. It puts too much reliance on remembering how this condition interacts with the one on the if statement above it, which can often be many lines away. I’d infinitely rather use a case statement where all the alternatives are in an easily-understood table with every condition aligned on successive lines.
Now, I do run the risk of having things change out from under me, but putting all of the defensiveness up front allows the happy path and some alternatives to be more succinct in the code.
I keep finding new things to be paranoid about, though!
if (!someCondition) { // short amount of code } else { // longer happy path }
The contextual overhead of having to invert a negative (i.e. !!someCondition) is just annoying.
I do agree that if (happy) { /* tons of code / } else { / I forgot how we got here */ } can also be an issue.
I think this is the first time I've heard that name for this idea. You'll find more under the term "guard clauses" - it even has its own Wikipedia page: https://en.wikipedia.org/wiki/Guard_(computer_science)
I'm with the parent poster, though: if you can use control flow to keep the happy path the unindented path, and you have syntax highlighting to help you show that each guard clause actually does interrupt the control flow, it can often be significantly cleaner.
I also like to think of it in terms of "railway oriented programming" (https://fsharpforfunandprofit.com/rop/ which is a must-read for anyone trying to wade into monads and functional programming IMO) - the "happy path" should be the path that it's easiest for the train to go down, namely a straight line down the page. Only give yourself the headache of a sharp turn when you're interrupting that path!
That said, your second code block is unreadable to me -- even knowing what it does, it's just confusing. No way that that's more readable than making use of break here. If you don't want so many conditionals in the loop body, you can factor the first check out:
The problem traditionally cited with goto is it can go anywhere (even into the body of another function where expectations on which variables are set may not hold).
If you don't like break and continue, how do you justify exceptions? The handling code for those may be in a different file or nowhere. Much closer to goto.
I accept your criticism, that my example is much harder to understand, and I admit that it's probably not the way to go in terms of maintainability. However, I'd still argue that my example is closer to an FP approach (even though it still uses a procedural loop), and that it communicates the logic in the same way that an FP approach does. In my example, it's clear that we're filtering out yellow (whereas with "continue", the logic is written in an inverted way); and it's clear just from reading the loop's header that we stop looping when rainbow is found (whereas with "break", logic to stop looping is being added at any arbitrary spot inside the loop's body).
Re: exceptions. I know that many more people object to them, than to break / continue (and whole languages make a point of not using them, most notably Golang). The pros and cons of exceptions is a whole topic in itself that I'd rather not digress into right here. Anyway, personally I'm ok with them. Firstly, they're for errors, and I feel that having weird control flow is sort-of justified for errors, whereas it's not justified for expected behaviour. And secondly, they're at least consistent in that they can occur anywhere, whereas break / continue can only occur inside loops (in Python, at least - in other C-family languages, "break" has different behaviour in different contexts, which is IMHO even worse).
I value being able to grasp what the code does at a glance while scrolling through. Your first snippet is entering my brain like as if without even reading at all, all at once and instantly.
Your second example absolutely does not and I'm sure you know why.
The way to avoid control flow here is to simply find the rainbow first, take the sequence up until then, filter by yellow, and thingify the remaining things.
Excuse formatting, cellphone.Not sure this is any better than the obvious solution either.
> it could push the task of precondition checking to its caller, and enforce via types (or an assert) that the precondition holds
(Emphasis mine)
When I began writing C# I recall my other team members "refactoring" my code to be shot through with multiple exits instead of a my collecting variable, because "it looked nicer." When they asked me why I wrote it the way I did, I mentioned the Java debugger thing, and they kind of looked at me blankly and said, "Huh, never thought of that."
Times change.
If you wrote the code to have a single exit, the cognitive load was also typically lower, as the control flow could be thought of as one way in, and two ways out (return or exception). Good disciple also followed "the jolly good idea of Demeter" or whatever it came to be called; that being to only operate on the state (or preferably, values) passed in, where possible.
In my experience, having multiple exits is even better because I can check which edge case is being hit by placing multiple breakpoints, or a breakpoint at a specific exit I want to check. I understand the pain of having to step through the code to get back to a certain execution point, but I’ve never been in a situation where multiple exits caused such cognitive load as to hinder that process further.
I'm not following you. Do you have a better example?
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...
[1]: https://macton.smugmug.com/Other/2008-07-15-by-Eye-Fi/n-xmKD...