Anti-If: The missing patterns (2016)
code.joejag.com
code.joejag.com
if !cache.contains(key) {
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
return v;
} else {
return cache.get(k)
}
by the time you get to the "else", your mental stack is filled up with the cache loading logic - you have to work to recall what this else branch is triggered by.instead, make the branch be the least complex option and exit early, and skip the else. That way, it can be read as "if this condition, do this tiny logic and exit", and that branch can then be mentally pruned.
if cache.contains(key) {
return cache.get(key)
}
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
return v;There are ways to do early returns in C without duplicating resource releasing code though, using gotos (it's one of the few cases where gotos make sense, IMHO). Code looks like this:
int aFunctionWhichMightFail() {
int result = FAIL;
Foo* foo = allocateSomething();
if (!someFunction(foo)) {
goto fail;
}
moreWork(foo);
result = SUCCESS;
fail:
releaseFoo(foo);
return result;
}
The Linux kernel makes heavy use of this.Gotos like that are fine. Despite the title of the paper I don't think Djikstra's meaning was that gotos should be removed entirely.
Like, imagine the spaghetti lovecraftian horror of a non trivial program that's just if and goto. At the time, there was a particularly vocal camp that essentially made the argument that it shouldn't matter since if/goto is equivalent to what exists in the machine code anyway, and you can technically do all the same things.
These things are hard to notice, and even harder to find elegant and effective solutions!
The point is to reduce the problem space so all you are left to look at, when you do come back to your code, is what's important to reason about without removing important details or adding trivialities. These problems haven't gone away just because the literal word goto isn't used anymore.
Such as?
I am aware of writing this in different ways, of architecting the logic quite differently. But the only control-flow construct I've seen as alternatives are to abuse exceptions or labelled while with named breaks.
What were you thinking of when you wrote this? I'm genuinely interested.
You can however give named return values and reference those names, and I believe it will work.
I've not seen any other constructs in any language that would come close, and I still maintain `goto fail` is a reasonable construct.
I'm actually not sure offhand how multiple deferred statements are handled. I could envision that the specification might make no explicit guarantee about the runtime order or it might create and pop off of a stack (at least in behavior).
Foo foo = acquireFoo()
defer releaseFoo(foo)
if !isValid(foo) {
return FAIL
}
moreWork(foo)
return SUCCESS
That is impressively clear. I don't use go, myself.I agree, I am quite comfortable using goto in C/C++ in this context. Though it may be better in C++ to use RAII in this specific example.
(with-open-file (fhandle path)
(if (has-record fhandle)
(process-record (get-record fhandle))))
Python basically has this, but Python doesn't give you the macros needed to write your own for other kinds of resource.http://clhs.lisp.se/Body/m_w_open.htm
> If a new output file is being written, and control leaves abnormally, the file is aborted and the file system is left, so far as possible, as if the file had never been opened.
Shades of PCLSRing!
Python doesn't use macros, true. But it absolutely gives you the ability to do 'with' for other kinds of resource (and anything else). Look for the with-statement and documentation on defining your own context manager.
The ease of writing macros such as with-open-file means that most libraries that need to have similar types of features for their resources that needs releasing also provides similar functionality.
try {
doSomething();
}
catch (Exception e) {
handleException();
}
finally {
cleanUpResources();
}
When the code exits the try block, you are guaranteed that it will run the finally block, possibly entering the catch block first if an exception was thrown. Since the most common use of `goto fail` pattern is freeing memory, the `finally` block isn't actually used a lot in Java/C# code in practice. Foo foo = acquireFoo();
try {
if (!isValid(foo)) {
return FAIL;
}
moreWork(foo);
return SUCCESS;
} finally {
releaseFoo(foo);
}
Python could also use the same approach. Though it also has context managers / the with-statement for this specific application.That there is no high-level equivalent, and Djikstra, while he sometimes had useful insights, was ultimately a self-righteous twit.
Sorry to disappoint; I would also be interested a (not-even-worse-than-goto-like-exceptions-are) answer to your question.
[0] https://www.cs.utexas.edu/users/EWD/transcriptions/EWD02xx/E...
There's just too much cleanup, such that avoiding gotos means you're either nested 200 columns into the screen or have half your function filled with partially-duplicated cleanup code.
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attribute...
So we ended up with a Best Practice of one return clause per function. Which is triply stupid because 1) long functions are part of the problem, 2) break to the end with multiple assignments to the same variable is almost as bad. It’s just trading one stupid for another which is a waste of time and energy.
And then for many years a loud minority of us, some of whom actually have a vague understanding of how the human brain processes data, insisted on the pattern you describe here. Some of us call it “book ending”. All the returns are fast or proceed to the end. Any method that can’t accomplish either has too high a complexity anyway and gets split until you get either one return, or book ends.
[another responder calls them guard clauses, but this doesn’t quite apply in all situations. A clause that handles a default or a bad input is why you can rearrange, but it misse why you should: ergonomics]
At the same time, C does not have any features for reliably managing resources. No GC. No C++'s RAII. Instead, you have to be careful to release every resource you acquire in the body of a function, along every execution path.
In that world, avoiding multiple returns makes a lot of sense because it ensures every execution path reliably reaches the end of the function so it's easier to ensure resources are released.
We aren't in that world anymore, so the guideline no longer applies. But it wasn't a stupid rule. It's just unhelpful to take it out of its original context and expect it to apply to a new one.
"Check your boots for scorpions before putting them on" isn't a dumb rule in the desert, but it is in Antarctica.
Makes no sense to turn your code inside out for resource management when you picked a language where you don't have to turn your code inside out for resource management.
I got taught this matter of style in the context of 90s C/C++. The general idea was that many return points likely means many point of having clean up any state you've been trying to manage/encapsulate with the function... in particular the state of manually managed memory. Which probably means mistakes.
It's a reasonable argument for manually managed memory, loses much of its power once you're not. And these days, most of us are not.
To me the second version makes it more difficult to differentiate the structure:
if (...) {
foo()
return ...
}
return bar()
from: if (...) {
foo()
}
return bar()
I find that when I'm reading code I'm generally more interested in the overall structure than in the specific implementation details. Especially since if the first block was complicated enough to tax my cognitive overhead, then I would probably extract it into a function anyways. function foo(bar) {
if is_null(bar) {
return TypeError
}
if !validate_input(bar) {
return ValueError
}
if cached(bar) {
return cache(bar)
}
// Added as a workaround for client XYZCorp since...
if (bar == "A special value) {
return "A special result"
}
return do_a_complex_thing_to(bar)
}
You could equivalently structure it as a bunch of else ifs, but there's no real reason to do it that way. if cache.contains(key) {
return cache.get(key)
} else {
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
return v;
}I also would like to add that it helps to fail early.
The article shows a null check for an add function. But the question is: why not throw an exception when the input is not what you expect.
This helps to find bugs very early.
return dbcache.getOrCompute(key, function() {
v = db.query("..")
expiry = calcexpiry(v)
return (v, expiry)
});I see these early returns as a list of assumptions. It helps making them nicely lined up, with perhaps some comments before each, and an empty line between them.
And then when you get below to the, now non-nested, code that actually does some work you have a smaller mental burden needed to understand what is actually done.
if !cache.contains(key) {
v = db.query("..")
expiry = calcexpiry(v)
cache.put(k, v, expiry)
}
return cache.get(k)
Only one return, and everything is still in the logical order.In your example above, the branch does merge back, and clearly that's fine for a small function - and common pattern - like this. But the general idea would be to avoid it if possible, because that removes unnecessary complexity.
One small problem is that depending on how your threading and cache works it might be possible for the cache to be cleared after the contains call but before the get.
That’s inefficient and, worse, introduces a concurrency bug. The cached value may expire between the check using ‘contains’ and the call to ‘get’.
A good API would have something akin to C#’s TryGet (https://msdn.microsoft.com/en-us/library/bb347013(v=vs.110)....).
plus this arrangement would seem vulnerable to a pathological case where you found the V from the db, but have a full/failed/partitioned cache and end up faulting or otherwise not returning the V.
Enums vs inheritance isn't straightforward. Enums make it easy to add new kinds, while inheritance makes it easy to add new methods. They are more extensible in different directions. If it's very rare for you to add new values to your enum, and more frequent for you to switch on that enum, prefer enums (and always throw in your default clause!). If you are adding enum values all the time, and only have a couple of methods that switch on them, then prefer inheritance. Although if you're using Java, consider methods on your enum values instead.
To understand the enum / inheritance tradeoff better, examine the Expression problem: https://en.wikipedia.org/wiki/Expression_problem
This is mostly solved in languages with named parameter calling (if you use it). Example: createFile(name, contents, temporary=True)
Sometimes it is already obvious enough. In C I have no problem with set_visible(item, true);
I’ve found the odds of that happening approach certainty once you have more than one or two parameters. If you’re using a language without named arguments I usually treat adding a third parameter as a cue to reconsider using a more idiomatic interface, whether that’s something like classic Java OOP (.setTemporary(true)) or limiting that knowledge to one place by e.g. creating a subclass, struct, or a specialized function so you call createTemporaryFile() and know that ordering check only needs to be done in one place rather than by every caller.
For example, you could mean
createFile(name, contents, /* temporary= */ true, /* overwrite= */ true)
but write createFile(name, contents, /* overwrite= */ true, /* temporary= */ true)
by accident.Writing documentation that that merely copies existing code is terrible: it's either going to be wrong (because it's not automatically checked and will go sour in a refactoring, or it's right because it's so obvious that no one will guess wrong when looking at it.)
int temporary = true;
create_file(path, contents, temporary); create_file(path, contents, temporary:=true);Unless you are using a different definition of named parameters, I've never seen C++ code in the wild that used them.
http://www.cs.technion.ac.il/users/yechiel/c++-faq/named-par...
Many languages now also have discriminated unions (enums with data), like F# and Rust, which make enum the better choice in more cases.
Even the 'case' expression is mildly deprecated in favor of pattern matching and guards in function heads (definitions).
There are no null values, no classes and no inheritance, but the presence of atoms (arbitrary tokens that can be used like null, true, false or enumerations), and pattern matching (data polymorphism and destructuring), means that conditional execution is elegantly implemented by functions.
This is the second, often forgotten, meaning of functional programming. Providing functions as first-class types, supporting anonymous lambdas (closures), and applying functions over collections are given all the glory, but deprecating conditionals in favor of pattern matching over many fine-grained functions is just as significant.
Unbound variables are the equivalent (detected if used, at compile time), but a much saner way to deal with value-less variables
Ifs exist because most real world problems require them. “If the box is checked, do this, otherwise do that”.
Masking if statements by syntactic sugar doesn’t serve any purpose in my opinion and if anything it makes the code more opaque, or worse, may force you to duplicate code...
I actually went into this with similar misgivings. A lot of "get rid of ifs" advice, particularly from OOP people ends up making things more complex IMO. However most of the advice other than the "Switch to Polymorphism" seem very down to earth and a simplification. Less about getting rid of ifs as if their mere presence was a stain on humanity and more how to rewrite what they are doing more clearly.
It seems the biggest thing OOP polymorphism has going for it is that it is "open", that is, the possible derived classes need not be known in advance. But, the fact that our toolchains use a separate link step, and that linkers remain primitive and link-time code generation is not widespread, is merely an historical accident. There's no a priori reason things have to be this way.
For example, an alternative would be if all of the source for an executable were in a single image file a la Smalltalk, and compilation consisted of converting this "database of code" into a single executable (no shared libraries!) in one big-bang compilation step (no object files, no static libraries!) You could even have something which syntactically resembled open polymorphism, maybe even using existing languages almost as-is, but instead of compiling down to type erasure and dynamic pointer casts, would compile down to Sum Types (variants). (The compiler could scan the image to find all of the derived types it could be and could construct a variant out of that.)
I'm not saying this is better, just putting it forth as an example of how things could be radically different than they are now (even for statically typed, compiled languages), had we made different choices. Consider the alternative, the situation we have now: we use a technique which could support anywhere from 1 to infinity polymorphic derived classes to implement interfaces which probably have approximately 2 different implementations in a typical finished program.
Perhaps an analogy would help: If we approached hardware the way we approach software, we would be using connectors which support between 1 and infinity pins everywhere when connectors which support N pins would suffice.
What polymorphism is good at is building the black box, the soft boundaries, and that isn't a good thing to do for fine-grained details. I do occasionally find a use for extension, but it's at a scale closer to "call this entire subprogram as a kind of state machine", vs "write this algorithm as a collaboration of objects abstracted away from each other".
The "alternate hard and soft layers"[0] pattern comes to mind: if what you really need is modularity, going towards full dynamism and reflection seems like a better choice than to try to extend everything statically, which creates a very large latency issue(the default response to every form of change in a static system is "recompile from the beginning, precompute all answers"). At the same time, it's not a good fit to have a vast codebase do everything dynamically since the comprehensibility and throughput suffer.
Your idea of replacing polymorphic variants with sum types sounds interesting. It also strikes me as quite similar to compile time polymorphism (e.g. generics) where you are 1 to infinity in the design space but the generated program boils down to only the variants created. Language wise I really like the mix of sum types, traits and generics in Rust.
It seems that the issue is less that the code is made more opaque but that you can quickly think of some counter-examples or nitpicks that make the advice less valid to you?
I think the problem of not being universal is true of most programming advice. So I don't necessarily see it as a reason to discount it. I also think in contrast to a lot of articles the author has done a great job to couch their advice as being contextual. For example for your criticism of Pattern 4 the author does actually point out the obvious solution to these complex expressions. To split them out into several parts. Which is what you get with if statements to an extent but spread out much more with a lot of clutter.
Some of your criticism also seems to be based on a misreading of the authors intent. For example Pattern 5 where the goal isn't to remove the if statement but prevent having to repeat the same error checking pattern everywhere.
My point is that this in itself is bad advice.
Overuse of oop concepts is just as harmful (if not more) as overuse of if statements.
But I'm fairly convinced the author isn't saying that they are to be generally avoided, to quote directly:
> If statements usually make your code more complicated. But we don’t want to outright ban them. I’ve seen some pretty heinous code created with the goal of removing all traces of if statements. We want to avoid falling into that trap.
The author's examples are not all about oop -- many of them having nothing to do with oop, they are just good advice for clean code in general.
fn box(status=checked)
do domething
fn box(status=*)
do something else
My Elixir projects have 0 to 10 ifs and they do solve real problems. I use plenty of ifs in other languages but I follow some of the advices of the post.Most pattern matching in languages is basically souped-up switch statements much of the time. Note that pattern matching is not one of the solutions mentioned by the author.
A Swift-enum allows you to have methods inside enums:
enum ConnectionStatus {
case connected
case connecting
case disconnecting
case disconnected
func `for`(isConnected: Bool, isConnecting: Bool, isDisconnecting: Bool) -> ConnectionStatus {
switch (isConnected, isConnecting, isDisconnecting) {
case (true, _, false): return .connected
case (false, true, _): return .connecting
case (true, _, false): return .disconnecting
case (false, false, _): return .disconnected
}
}
Then somewhere inside your code: public func connect() {
switch ConnectionStatus.for(isConnected: self.isConnected, isConnecting: self.isConnecting, isDisconnecting: self.isDisconnecting {
case .connected, .connecting: return // We're already good
case .disconnecting: reconnectWhenDisconnected()
case .disconnected: startConnection()
}
The amount of if's that have to be juggled to do the same and the resulting code that is hard to parse when you're maintaining it definitely is more problematic.Same goes for `Optional<T>` in an argument position, though https://github.com/quchen/articles/blob/master/algebraic-bli...
This blog post suffers from the classic problem of blog post code examples - to fit the example in a blog post it needs to be trivial, and because it is trivial it doesn't demonstrate the real value of these techniques.
I see if/else overused all the time - every single day in fact - when the techniques in the blog post should have been applied.
It was then my job to translate that into SQL which doesn't really have "if" in the same sense as an imperative language does.
Sure I could have pulled out all the data and done the branching in Python, but that would have been a performance hit, but mainly because it would be ugly, require a lot more code and likely be a lot buggier. These days I try to keep as much work done in the database because the declarative nature of SQL generally works out with a lot less bugs as well as the performance benefit.
Sure it's not going to be possible for every case, but I have noticed that the more experienced I get, the less I like "if"s.
If your code contains quite a lot of branching - especially the same pattern of branching in several places - then you should certainly consider replacing that with polymorphism, in the form of virtual methods or perhaps templates/generics (compile-time polymorphism). But you should also consider leaving the branching in place if that is simpler overall. That doesn't make the article bad advice IMO, you just need to not take it too seriously.
It’s annoying though (and kind of strange, really) that these articles (which discuss very basic concepts) make it to the front page, and may influence a bunch of rookie developers the wrong way.
As design theory? Fine.
As HN frontpage stuff? I mostly see it get discussed in the form of new devs or students asking things like "wait, how should I remove all the 'ifs' in my Java code?" (Yes, that's a real example.)
Spreading the good news about case statements, function passing, and so on is great. But this stuff is so often written up as absolutes, when simple conditionals really are a part of most practical cases.
As a first step, I'd say better to just isolate the branching in one place, as a separate function, rather than immediately jump to polymorphism which could have high refactoring costs in some cases (even if it might eventually be worth it).
Because premature abstraction is just as bad as premature optimization.
There's a notable difference between "if the box is checked, do [thing 1], otherwise do [completely different thing 2]" and "if the box is checked, do [thing 1], otherwise do [basically thing 1, but with this additional metadata from service y]."
And that difference, I think, is what tells you where to put the `if` statements. If things are distinct enough, you have some easy-to-read top-level ifs and switch based on that. Otherwise it's more complicated to trade readability vs code reuse/deduplication.
But where you get into big trouble is when you don't think about the dangers of if's at all, and end up with call stacks 5+ methods deep with no rhyme or reason to why certain things are done in if statements in level 5 and other things are done with if statements in level 1. Creating a mental map of what's done where in what conditions for that kind of code is really hard - as is thoroughly testing it, since this often means you're passing a lot of context really deep and don't have easily separable units. (And yeah, "Switch To Polymorphism," I think, is a risky one here - that can turn into "still have the deeply nested if statements, but make them invisible.")
I don't think the article really hits that well, though.
My attempted better advice would be to keep methods short and the nesting/indentation limited. Use good method names. Make it easy to understand each method in isolation.
Seriously are you saying that
if (x) {
return y;
} else {
return false;
}
is as easy to read as return x && y; <?php
$x = 1;$y = 2;
function test1($x, $y) {
if ($x) {
return $y;
} else {
return false;
}
}
function test2($x, $y) {
return $x && $y;
}
var_dump(test1($x,$y));
echo "\n";
var_dump(test2($x,$y));Since otherwise, it’s not a question of readability anymore. (But yes, incorrect transformations are incorrect.)
if isAdmin:
return false
if not isActive:
return false
return true
vs return (not isAdmin) and isActiveRegarding which way is better, there's not much benefit arguing one way or the other. This is a leaf decision. It doesn't affect the structure of other parts of the code.
It's been in nearly all languages, and for decades before python existed.
>>> 0 and 'asdf'
False
>>> 3 and 'asdf'
'asdf'
>>> 3 and ''
''
'and' and 'or' in python are short-circuited as in e.g. C, but it's even more short-circuited! The "last" component is not evaluated in a boolean context. For a complete evaluation you have to use the expression in an if or while statement or apply the bool() function. I would say it's like a monad, if that helps. if (a) return false;
if (!b) return false;
return true;
May be easier to read than: return !(a || !b); !a && b
? :)a -> isAdmin
b -> isActive
const isDeletableUser = !isAdmin && isActive;
return isDeletableUser;
(Better would be naming the function "isDeletableUser" instead of the intermediate const, but was keeping with the original.)Point is for me that booleans with readable names shows how eliminating ifs becomes not only doable but arguably preferable.
return a ? false : b;I’m a heavy user of ternary, but it’s still an “if”. And and Or are (not very much) higher-level operations.
The overall logic remains the same, it's really just giving one a clearer view of a specific path.
You need to look for a balance between ifs and alternative patterns.
I like this statement. After 30 years of coding, I've found that trying to do something quick/clever without naming it explicitly, for fear of letting the code get bigger, is what often leads me to write confusing code and harder to understand ifs.
I think maybe it's that naming in new code is easy, but adding new concept names to existing code as it grows gets a lot harder. I'm automatically naming when I create a new class, but when I fix bugs or touch existing files, I'm actively trying to avoid naming things and I'm trying to touch the least possible code, so over time it trends toward having unnamed concepts.
I usually try to apply naming to Pattern 4. So maybe instead of:
return foo && bar || baz;
I might: bool bad = foo && bar;
bool worse = baz;
bool isHorrible = bad || worse;
return isHorrible; public boolean foo() {
if (!this.doesSomethingWithA()) {
return false;
}
if (!this.doesSomethingWithB()) {
return false;
}
if (!this.fooBar2000.isEmpty()) {
return false;
}
if (!this.anotherLongAttribute) {
return false;
}
if (!this.anotherMethod()) {
return false;
}
return true;
}
We try to replace to a "one liner" return !A && !B && !C ... Well, the expression was very long and we noted that was more hard to read that the multiple if's. I suggested split the one liner expression on multiples lines doing something like this : return !this.doesSomethingWithA()
&& !this.doesSomethingWithB()
&& !this.fooBar2000.isEmpty()
&& !this.anotherLongAttribute
&& !this.anotherMethod();
However, the code formatter (eclipse), changes every time it to the hard to read one liner expression. So we ended using the multiple if's.First, consider this reworking:
boolean a = this.doesSomethingWithA();
if (a) {a = a && !this.doesSomethingWithB();}
if (a) {a = a && !this.fooBar2000.isEmpty();}
if (a) {a = a && !this.anotherLongAttribute;}
if (a) {a = a && !this.anotherMethod();}
return a;
And then this one: boolean a = this.doesSomethingWithA();
a = a && !this.doesSomethingWithB();
a = a && !this.fooBar2000.isEmpty();
a = a && !this.anotherLongAttribute;
a = a && !this.anotherMethod();
return a;
In the first version and your original, it's clear that there's short circuiting and it can return early, but automatic formatting will bloat up the vertical size of the code. In the one-liner there's also short circuiting, but it doesn't format well. In my second version you get the formatting, but it will always evaluate every possibility.In general, when I'm favoring code formatting, the variable declarations come out. Quickly aliasing something into a name when it could exist purely as an expression adds a degree of conceptual flexibility. It keeps the code local(no new function name and jumping over into it). But it does also result in this kind of unnecessary computation.
Total tangent, but I'm very much looking forward to more and better alignment rules becoming pervasive. I love it when similar things line up, it's easier to read, easier to modify, easier to see mistakes, better supports column selection and multi selection. I'll take alignment over almost any other code formatting issue that programmers debate endlessly. ;)
Having a code formatter is a greater good though, so yeah sometimes multiple ifs is just a necessity. At least the original version is clear and easy to follow.
I would much rather see
return foo && bar || baz;
and a comment explaining behavior (although perhaps parens so I wouldn't have to reason about order of operations)With such polymorphism it's difficult to know the things that can happen. Implementations of the virtual method are not closed - they could come from anywhere in the codebase.
Another problem is that it's hard to cleanly separate the common code from the specialized code in each implementation class. That's why one ends up with ugly helper functions that receive a lot of state, or alternatively with unmaintainable class hierarchies.
IME the only solution is to minimize usage of datastructures that have choices in them ("ADTs") and minimize the coupling of the code to the choices. The latter is done by separating code paths early, and by only switching if absolutely necessary.
Helper functions that receive a lot of state aren't ugly, they are pure. Pure code is easily tested. It is easy to reason about and reuse, because it's clear what the expected inputs and outputs are.
(This is not a hard rule. For example, layouts for disk serialization might be better done as a single union with a discriminator field. In this case we're not optimizing for code but for disk accesses).
IF's often handle unpredictable variations better. Features are often better viewed as a buffet restaurant instead of an animal kingdom classification tree as found in biology. The biology classification-like approach popular in the 90's usually fails outside of biology. And, "lots of little methods" can make code rather hard to read & follow in my opinion. Your eyes may differ.
The real world is an undifferentiated unified whole, but you can't actually work with it without dividing it up and labelling/modelling it, and most useful models have a heirarchical structure. OTOH, they usually aren't a single-inheritance heirarchies, and most OOP languages either handle multiple inheritance poorly or not at all.
It's the approach taken by the relational model, used in RDMBSes.
Let's look at UI's. Should "buttons" and "hyperlinks" be considered different "types" of things? Or should we blur the distinction to be flexible? In this view, a button is a style applied to a hyperlink rather than a distinct "kind" of thing. Once you pre-type them into distinct things, it's hard to merge them: the code base hard-wires a clear distinction and too many things rely on a type-centric arrangement.
A more flexible approach is to take the view that things, perhaps ANY thing, wants the ability to "act like" a hyperlink and/or button. Linking is then viewed as an optional feature, not part of some base "type". Same with drop-downs versus combo-boxes versus text boxes: we can model these via feature smorgasbords instead of sub-types and probably get a more flexible framework in the process that may also be reusable in other widgets. But the flip-side is that such flexibility can confuse newbies. Hard-wired hierarchies are quicker to learn, and test.
And then what? This only works if you can eliminate the "choice" altogether. Otherwise the "choice" still will be encoded if your system, be it much less obvious.
Think using null/nil instead of an optional type. Now everywhere you use the value (including when passed to other functions) you have to test the value for not being null. With a proper ADT (Maybe, Optional, what you prefer) it is obvious when you have a value and when you don't know.
> you have to test the value for not being null
Don't assume possibility of null if it's not a valid value in your data schema. There are so many codebases rotten by never ending null checks. There are no benefits from them, and only drawbacks: They confuse the reader by checking for a situation that should never happen, giving the impression it could be a valid situation. (This is actually a point the OP makes).
Let me exaggerate a little bit. Superfluous null checks are similar to something like
def add(int x, int y) -> int:
return x + y
int z = add(1, 2)
if z != 3:
return 0
else:
return 3
Just because it's "possible" that z is not 3 because there are other values that are also integers, that doesn't mean you have to cover them. Type systems are much less helpful than many FP advocates want you to think, and if you depend too much on them you get a bad, bad codebase (and you deserve it).The main benefits that type systems bring: 1) they catch your typos early, 2) they support compilation to efficient machine code.
If you use tables and indexes, there are no pointers and no null anyway. Then again, I've used -1 as a sentinel in the past, and there are many more unused negative values available if you need them :-)
Having structure in your programs is of utmost importance. I just don't think type systems can capture all of the programs' finer points. And if they could, why would we write any program code at all?
Why are relational data schemas a good idea? Because they are simple. They bring clear benefits and rarely any problems for implementation. (I mean, if we constrict ourselves to simply record definitions: They are basically a physical description of your data, so can hardly be a constraint).
Anywhere in the codebase that satisfies the interface (or inherits from some base class) -- which is trivial to find, and what you want.
Who said they had to be closed?
And what if you need a plugin architecture for example?
A plugin architecture is an entirely different kind of beast, and rarely needed. It requires some sort of interface to achieve the decoupling. Inheritance is one option here, albeit an ugly one.
Well, they could be implemented in any class that derives from the ancestor class that declared the virtual function. Unless you have one main base class, that should be a very small subset of the code. What's more, it has to have the same name. A recursive grep should show every implementation in the codebase.
In terms of ergonomics, I'd say having to recursively discover implementations is quite a stretch from reading a simple switch block that contains all the possible continuations case-by-case. Psychologically, obstacles like this can prevent you from maintaining the code.
For example, the advice "Pattern 2: Switch to Polymorphism" definitely does not hold in languages with algebraic data types, where the compiler will check that your "switch" expressions check all possible cases. Given that at least Rust, Swift, Scala, and Typescript can all do this, this kind of advice is becoming increasingly outdated.
As a matter of fact it does. Polymorphisms come in many forms (pun intended). The OO-pattern of class inheritance is simply just one of them - not quite so good too if you ask me :)
The author seems to believe that polymorphism is OO-specific. But it isn't. As a matter of fact, if/switch statements are _also_ polymorphic, often used in more procedural code.
The more functional way to match on different type constructors is a fine example of a polymorphic function.
Polymorphism has a straightforward meaning, that the same code is operating on different data types. Switch statements (if's are just a special case) are running different code on different types, so they're monomorphic.
> The more functional way to match on different type constructors is a fine example of a polymorphic function.
I think I see where you're coming from. Here's an example of what you're talking about.
data Foo = Foo Int | Bar String | Qux
what :: Foo -> Foo
what (Foo num) = Bar $ show num
what (Bar str) = Qux
what Qux = Foo 0
That's not polymorphic; there's a single type there, and any given code is strictly acting on a single type of data. It's just a nice way of writing a case.And, in fact, you find out it's not magic because it gets just as messy and confusing once you have to nest logic as with imperative programming, for example, if Foo contained another ADT, I'd often have to use a case statement or equivalent to manage it.
I agree with everything else you've said.
(For the pedantic: one can argue about the definition of polymorphism. "Type constructors" do not create distinct types in an algebraic data type as implemented in ML and Haskell, so arguably destructing an algebraic data type using pattern matching is not an instance of polymorphism as the input type is not varying. However that is not germane to this discussion.)
No If-statements in code reminds me a bit of the E-Prime movement (https://en.wikipedia.org/wiki/E-Prime) which was about removing the verb 'to be' and all its variants like 'is', 'was' etc from English language. Supposedly it made English easier to understand and encouraged clarity and precision.
Right, and you should try and make that decision as early and as few times as possible. For example, let's say you work with older systems that have grown. You went from one type of config to two types, so now you have If (configOne) ... else ... scattered all over your code. You can't really get rid of the decision, it has to be made, but you can refactor the code to make the decision only once when the app starts up.
Surely I can't be the only person who finds
if (foo) {
if (bar) {
return true;
}
}
if (baz) {
return true;
} else {
return false;
}
Much easier to parse than return foo && bar || baz;
It's a bit better to my eye with parentheses: return (foo && bar) || baz;
Of course the first version is far from perfect (no need for the nested if, etc.) but I've always found too many comparisons on one line just looks messy. return (foo && bar) || baz;
The most readable.The top version just has too many curly braces and parentheses. I can keep each branch in mind easily, but keeping the whole program flow at once requires me to first parse it and translate it.
The last version, I can read as "either baz, or both foo and bar", which is easy to keep in mind at once, and would be much easier to pattern match for errors ("Oh no, I wanted foo and baz, not foo and bar") than the top version, for which I would possibly have to iterate each potential execution path in my mind to spot a mistake.
Consider, more concretely, (since I can't think of a better example), knowing whether to give someone an alcoholic drink. Let's go US style. Either they order a soda, or they are old enough and have ID. Three variables: isOver21, isSoda, isIdPresent.
(Putting aside the fact that the order here is weird and it could be designed better) we can either go:
if (isIdPresent) { if (isOver21) { return true; } }
if (isSoda) { return true; } else { return false; }
Or we can go: return (isIdPresent && isOver21) || isSoda;
Now what if we accidentally swapped two variables:
if (isSoda) { if (isIdPresent) { return true; } }
if (isOver21) { return true; } else { return false; }
vs
return (isSoda && isIdPresent) || isOver21;
Note how in the first case, if I am tired, I could miss it. Most of the code paths seem correct. If someone is ordering a soda and has an ID they should get the drink. If they are not ordering a soda, and they aren't over 21, they shouldn't get anything. If they are over 21, they should probably get something.
In the second case, I can see all the logic in one line, and I can say "hold up, why are we asking for soda and ID, or age? Something seems off." Not that I can't do that with the multi-line if, just that it might take a bit longer.
Source: I've been bitten by multi-stage if statements on a few occasions.
I've been bitten by that before and thinking through all the possibilities when using multiple ifs is harder to me than with a boolean expression where a truth table can be used as a formal proof of validity.
The other downside to ifs is that the order of the ifs matters. That makes the code more brittle and requires the reader to parse the code on two levels: thinking about the booleans and keeping their values in mind over multiple lines of code. In a short sequence it's not a big issue, but it can be annoying when a very distant if statement affects your reasoning much later. Of course order matters without ifs, but everything important is on one line.
So the boolean version keeps all the relevant reasoning in one place -- no need to scan multiple lines of code (and risk missing an important guard).
But if you're just trying to condense down separate business rules, where it happens that today that you want the same thing in the case of `baz` as for `(foo && bar)`, then you're setting yourself up for a more painful change when someone comes to you and says "hey actually let's have `baz` do the other thing after all. Yeah, in the trivial case where it's just returning bools, that's not really an issue, but when I see code that looks like the former it's usually more complicated, and the overlap is more often incidental rather than fundamental.
So my approach is more "remove nested ifs" than "simplify it into as few cases as possible."
`if (foo) { return true } else { return false }`
is the same as `return boolean(foo)`
Hard to say how to properly refactor out the if statements without knowing the problem domain. And I guess that's basically the point - using if statements means that you are writing imperative code rather than modelling the problem. You are specifying to the compiler how you want things done, rather than what it is that you want done.
Moving from 11 lines to 1 line is, for me, vastly superior readability. And anyone who has been programming for a couple years at least would have no problem parsing the order of operations and precedence between && and || (logical 'and' is always higher precedence than logical 'or' in every language I've seen), but parenthesis are a trivial addition if desired.
void blah(int x, bool logInfo) {
...
if (logInfo) ...
...
}Pattern 2 can be horrible when applied to the wrong problem. As in: you can use polymorphism to implement fact(int x) without if statements, but it’s not going to be pretty...
Pattern 3 is not a pattern, it just says “if the if is useless remove it”...
Pattern 4 is particularly confusing, especially when you start to have nots in there:
var x = !(a || !b) // ugh
Pattern 5: doesn’t remove the if...
if(log) { logger.write(computestuff()); } logger.write(computestuff);
Or for parameterized functions: logger.write(() => computestuff(a, b, c));
Then all that logging policy stuff can live with the logging and doesn't need to be checked explicitly throughout the codebase.By addressing it via (well, I'd use an interface or similar) inheritance the conditional logic isn't removed, but it is isolated to a few decision points. All you need to know is that you have a Foo, and my implementation whether it's a Bar or a Baz will provide the needed functions.
Similarly, in OO-languages with interfaces, you can apply this same approach. It doesn't eliminate the conditional, it still exists in the code. But it moves where you have to think about it and reduces the redundancy across your code base.
With these things you don't even have to modify every function when you add a new value to your equivalent to an enum, you just add the implementations for that new type/class.
Though it does shift the difficulty when you want to add a new function/method to the interface. Now you will have to go to each implementation and add this new capability. But the nice thing, here, is that most languages with static typing (whether fancy dependently typed languages like Idris, or relatively simpler ones like Java) will make it immediately clear which implementations are incomplete.
With tools like JaCoCo we track number of branches and cyclomatic-complexity in our code (which is a measure of IF statements in most cases, try/catch being the other big source of branches). We try to keep that number down to 1 as much as possible. Once was able to take a method with over 160 branches down to 1 by eliminating every IF statement. The unit test was then very trivial and only had one test.
His ideas changed the way I think about clean code.
* http://www.gar1t.com/blog/solving-embarrassingly-obvious-pro...
* http://www.gar1t.com/blog/more-embarrassingly-obvious-proble...
Brilliant!
> being defensive inside your codebase probably means the code that you are writing is offensive. Don’t write offensive code.
There are some catchy insights here.
Moving on to the actual point, for a balanced view just look at cyclotomic complexity. In a function, count the decision points. Every if and loop counts. Any function with 10+ decision points is too complex and needs to be refactored to make the control flow more obvious.
Following this rule will let you use if quite happily, while also avoiding the problems with it.
(A side tip. If your language has some variant of "unless", don't use it. Ever. Our brains do not do de Morgan's laws naturally. So you can write code that is clearer upon writing, but is much harder to keep straight when you're debugging. The time that you need to understand your code best is while debugging, so ease of debugging is more important.)
For example the switch on type: wouldn't it be nice you could simply have a compiler error whenever you forgot to either add default or added all values? I use enums a ton in other languages that do and it's like a to-do list of code to implement if I would add for example another specie. Java just doesn't so the only compile-time safe way to have it is to force subclassing.
Equally the argument about not checking nulls inside your own application. Nullable pointers are a language problem. The argument to that function should not be nullable to begin with. It doesn't make sense to call it with null. Ever.
I find foo && bar || baz is pretty hard to parse as well. It might be easier to throw all three arguments in a switch / case statement and give all relevant states their own case. But that doesn't work in Java.
The hidden "null means something" example could easily be rewritten to a response that simply has a Error or Result state that either contain the error or the result we wanted to have. Java only allows you to return an object that you need to cast to use (Error extends BaseResult, Result extends BaseResult) or declare it might throw a particular type of error.
I think all if not most of these problems are solved in Kotlin. Java is just not a great language to express problems in a short and succinct way without introducing side effects or massive amounts of boilerplate (sub sub sub sub classing).
more experienced in Swift but Kotlin devs can follow the patterns I use
[0] https://www.infoq.com/presentations/Null-References-The-Bill...
If you have sixteen polymorphic methods in one service class (which is of course possible), maybe using a switch statement would actually be a bit shorter and easier on the eyes. It depends. I think about this in terms of unit tests and code paths. More ifs means more paths. Units with lots of code paths are complex and hard to test. On the other hand having code paths cross many classes and methods just moves the problem. It's a tradeoff. A few other tricks you can apply here is using enums with e.g. lambdas and other feature flags. That's a nice way to tie functionality to types or associate some default functionality with domain classes without littering them with logic.
Not using abstract classes is somewhat controversial for some but I have disliked them for quite long and find I don't need them and can trivially refactor them away whenever I encounter them in an codebase. Preferring delegation over inheritance is a good thing IMHO and using inheritance generally is something I end up regretting. I find framework code (Spring, cough) that uses abstract classes to be mostly unreadable and convoluted. You have a Thingy that is also an AbstractThingy that inherits an EvenMoreAbstractThingy that implements a gazillion interfaces that are also implemented by other thingies and yet more AbstractClasses, etc. If you have to control click through six classes to actually get to a place that actually has logic, something is deeply wrong. Deep inheritance hierarchies are a good sign your type system is struggling with your abstractions and sets you up for maintenance problems when the cohesiveness of your classes drops and coupling increases as you are forced to add stuff in places where it really shouldn't be for API compatibility reasons.
> Context: You are calling some other code, but you aren’t sure if the happy path will succeed.
I’d love to see some good examples of this if-less code in React.
I find myself writing conditionals in data provider component render methods for things like “isLoading”, “hasError”, and “items.length == 0”.
That doesn't work if you are building a library, or working with other software developers. The alternative is to have a strictly typed language that forces the user to pick the type of the variable.
It doesn't also fix the problem of whether you function is returning a functioning result or an error.
That's one of the thing that I like about idiomatic Rust. The Result/Option types are implemented as standard language features. You get to use them from the get-go and they reduce lots of if/then/null friction.
Sometimes we might want to use "null" rather than an empty collection to signal something different i.e null -> error, empty collection -> no result (avoiding the debate on whether we should use some exceptions for all error management).But generally speaking, creating a new object everytime instead of passing/checking null looks like a real potential loss of performances in some (not so rare) cases.
It’s incredibly like Python exception handling: it works well until you try to work with a somewhat complex codebase: eg paramiko. The IOErrors can be raised by any layer of the stack. So how do I handle this in the code?
Well, turns out I can’t definitely tell one from another.
Same is with the null values: never use nulls as a flag unless you’re absolutely sure that null value will not be boxed in another “Optional”.
Oh and, I agree about the not using null thing at multiple levels.
Pattern 2: Yes absolutely
Pattern 3: Seems a bit inefficient, allocating memory when a simple if statement would avoid calling the method at all.
Pattern 4: Would prefer
if (foo && bar) { return true; }
return baz;
Than the proposed solution which is harder to read.
Pattern 5: Just no! The repository getRecord method should have no care about coping strategies, which may change depending on the calling context. It should instead throw an RecordNotFoundException. You could also use success and error callbacks in the method signature.
[1] https://docs.oracle.com/javase/7/docs/api/java/util/Collecti...
If link doesn’t open.
(Of course, debuggers are useful too, but code that requires them is not good.)
Imagine a combinator that takes an array of curried functions that are composed from a filter or reducer. No if statement needed if you can dynamicly compose a functional pipe
public class FileUtils {
public static void createFile(...) { .. }
public static void createTemporaryFile(...) { .. }
}
is that there is now no way to create a single wrapper function.Let's say you want to create a function "createFileAndLog" which takes the boolean parameter and passes it to the underlying "createFile" function, now you have to create two "createFileAndLog" functions rather than just one, even though you didn't have the if statement in your code.
I haven't provided a great example, I know, but I've definitely split up such methods myself, felt good about it for increasing readability, then come to regret it later.
IMO, if you are you should disclose that when you write a comment. I am all for people promoting their stuff — as long as they do so in a way that make it clear that they stand to gain from sales.