91–100 of 209 posts
I strongly disagree with this ifs take. I want to validate data where it is used. I do not trust the caller (myself) to go read some comment about the assumptions on input data a function expects. I also don't want to duplicate that check in every caller.
I really don't think there is any general rule of thumb here. You've really got to have certain contexts before thinking you ought to be pushing ifs up. I mean generally, you should consider pushing an if up. But you should also consider pushing it down, and leaving it where it is. That is, you're thinking about whether you have a good structure for your code as you write it... aka programming. I suppose you might sa…
This is not to say we shouldn't be having conversations about good practices, but it's really important to also understand and talk about the context that makes them good. Those who have read The Innovator's Solution would be familiar with a parallel concept. The author introduces the topic by suggesting that humanity achieved powered flight not by blindly replicating the wing of the bird (and we know how many such attempts failed because it tried to apply a good idea to the wrong context) but by understanding the underlying principle and how it manifests within a given context.
The recommendations in the article smell a bit of premature optimisation if applied universally, though I can think of context in which they can be excellent advice. In other contexts it can add a lot of redundancy and be error prone when refactoring, all for little gain.
Fundamentally, clear programming is often about abstracting code into "human brain sized" pieces. What I mean by that is that it's worth understanding how the brain is optimised, how it sees the world. For example, human short term memory can hold about 7±2 objects at once so write code that takes advantage of that, maintaining a balance without going to extremes. Holy wars, for example, about whether OO or functional style is always better often miss the point that everything can have its placed depending on the constraints.
Code complexity scanners⁰ eventually force pushing ifs down. The article recommends the opposite: By pushing ifs up, you often end up centralizing control flow in a single function, which has a complex branching logic, but all the actual work is delegated to straight line subroutines. ⁰ https://docs.sonarsource.com/sonarqube-server/latest/user-gu...
The way to solve this is to split decisions from execution and that’s a notion I got from our old pal Bertrand Meyer. if (weShouldDoThis()) { doThis(); } It complements or is part of functional core imperative shell. All those checks being separate makes them easy to test, and if you care about complexity you can break out a function per clause in the check.
I strongly disagree with this ifs take. I want to validate data where it is used. I do not trust the caller (myself) to go read some comment about the assumptions on input data a function expects. I also don't want to duplicate that check in every caller.
[0] https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-va...
I agree that the first example in the article is "bad"... fn frobnicate(walrus: Option )`) but the rest makes no sense to me! // GOOD frobnicate_batch(walruses) // BAD for walrus in walruses { frobnicate(walrus) } It doesn't follow through with the "GOOD" example though... fn frobnicate_batch(walruses) for walrus in walruses { frobnicate(walrus) } } What did that achieve? And the next example... // GOOD if condition…
> What did that achieve? An interface where the implementation can later be changed to do something more clever. At work we have a lot of legacy code written the BAD way, ie the caller loops, which means we have to change dozens of call sites if we want to improve performance, rather than just one implementation. This makes it significantly more difficult than it could have been.
Firstly, in many cases the function needs to serve both purposes — called on a single item or called on a sequence of such. A function that always loops would have to be called on some unitary sequence or iterator which is both unergonomic and might have performance implications.
Second, the caller might have more information than the callee on how to optimize the loop. Consider a function that might be computationally expensive for some inputs while negligible for others — the caller, knowing this information, could choose to parallelize the former inputs while vectorizing etc. the latter (via use of inlining, etc.). This would be very hard or at least complicate things when the callee's responsibility.
// Good? for walrus in walruses { walrus.frobnicate() }
Is essentially equivalent to
// BAD for walrus in walruses { frobnicate(walrus) }
And this is good,
// GOOD frobnicate_batch(walruses)
So should the first one really be something more like
// impl FrobicateAll for &[Walrus] walruses.frobicate_all()
Code complexity scanners⁰ eventually force pushing ifs down. The article recommends the opposite: By pushing ifs up, you often end up centralizing control flow in a single function, which has a complex branching logic, but all the actual work is delegated to straight line subroutines. ⁰ https://docs.sonarsource.com/sonarqube-server/latest/user-gu...
Code scanners reports should be treated with suspicion, not accepted as gospel. Sonar in particular will report “code smells” which aren’t actually bugs. Addressing these “not a bug” issues actually increases the risk of introducing a new error from zero to greater than zero, and can waste developer time addressing actual production issues.
Code complexity scanners⁰ eventually force pushing ifs down. The article recommends the opposite: By pushing ifs up, you often end up centralizing control flow in a single function, which has a complex branching logic, but all the actual work is delegated to straight line subroutines. ⁰ https://docs.sonarsource.com/sonarqube-server/latest/user-gu...
Code scanners reports should be treated with suspicion, not accepted as gospel. Sonar in particular will report “code smells” which aren’t actually bugs. Addressing these “not a bug” issues actually increases the risk of introducing a new error from zero to greater than zero, and can waste developer time addressing actual production issues.
Fiddling with the default rules is a baby & bathwater opportunity similar to code formatters, best to advocate for a change to the shipping defaults but "ain't nobody got time for that"™.
Moving preconditions up depends what the definition of precondition is. There's some open source code I've done a deep dive in (Open cascade) and at some point they had an algorithm that assumed the precondition that the input was sorted, and that precondition was pushed up. Later they swapped out the algorithm for one that performs significantly better on randomized input and can perform very poorly on certain sorte…