Avoid Else, Return Early (2013)
blog.timoxley.com
blog.timoxley.com
Same way you evolve out of one liners.
Same way comments are extra weight that should only be in public or algorithm/need to know areas.
Same way braces go on the end of the method/class name to reduce LOC.
Same way you move on from heavy OO to dicts/lists.
Same way you go more composition instead of inheritance.
Same way while/do/while usually fades away, and if needed exit conditions.
Same way you move on from single condition bracket-less ifs. (debatable but more merge friendly and OP hasn't yet)
Same way you get joy deleting large swaths of code.
and many others on and on.
Usually these come from hours of writing/maintaining code and styles that lead to bugs.
I couldn't interpret this sentence.
For one thing you can't misplace ";" in a while loop.
I agree that "do while" loops are unintuitive and rarely used, and thus the place to first check for bugs. Also they save very little so I just implement them as do(); while usually.
But I know people who disagree about that, too, and shout at me for avoiding them:) It seems what's unintuitive for me isn't so for others.
But yes if you end up in a loop you have some sort of counter/null checks or way out if you end up in a constant/recursive loop.
Never leave open the possibility of a lock due to being stuck in a loop.
I think for loops should be limited to iteration over collections, or repeating sth known number of times.
There are cases when you need to decide if you want to continue iteration on the fly, and while loop is the perfect tool for that. Also while loop is arguably less error-prone than for loop: can't swap clauses because there's just one, can't write "," instead of ";" by mistake.
Among all types a 'while' loop gives the strongest mathematical guarantees. In particular it ensures a condition is true at the start of each iteration, and is false when exiting the while loop. Combining this with loop invariants leads to precise and straightforward to verify code.
Avoiding while loops to ensure an exit just seems to be sacrificing a lot of power in exchange for some pretty shaky guarantees that your loop will finish.
Although in the majority of cases where a while loop could be used you're simply iterating over some (implicit) data structure. In which case it's obviously better to make this explicit using a "for ... in" loop.
What about error handling? No mention here. What I do not is let the library/framework to handle most of it. With a recent server library I wrote, I can throw anywhere and it will be caught and displayed as expected without extra try/catch.
OO is better if the objects tell a story in a way that ProviderStrategyDataFetchers fail to do so and are effectively just wrappers for data structures. If you go pure data structure without story you just end having to add comments to explain purpose. The comments are the classes.
You could argue the placement of braces with lots of valid arguments both way, but this... to reduce LOC ? Doesn't feel like a valid reason in any language using braces...
uh ? for me the less code on each line and the easier it is to read
Sometimes more verbose code is more readable, and sometimes it is the presentation, and sometimes this is "superfluous lines", that increase LOC.
The argument against placing braces on their own line is that this:
if (a)
{
print(a)
}
conveys exactly as much information as this: if (a) {
print(a)
}
while taking up more space. The thing the brace tells you is already told by the indentation, so the brace is on a superfluous line.Which is true, and of course begs the question: why do you even need the braces?
2) I semi-frequently make indentation mistakes when resolving merge conflicts. In braced languages this is fixed by an autoreformat. In unbraced languages, I have to take a lot more care with merging, lest I end up in yet another lengthy debugging session ending in facepalms - or worse, check it in.
3) Redundancy in the face of typos and whitespace destroying mediums such as many pieces of web technology - comments sections, forums, etc.
if (a) {
print(a)
}
and
if (a)
{
print(a)
}Could just be if (a) print (a)
Except in perl, where it's
print (a) if (a)
And if you want to add a second line you have to change it, thus I got into the habit of if (a) { print(a) }
But I don't code for a living, any more than I run network cables for a living, or screw things into bays for a living. I code as a tool to get the job done, most recently that was writing some perl to parse the output of a tcpdump which was outputting rtp sequence number discontinuities to see where packet loss was occurring, what jitter was going on, etc. Fast, to the point, isolate the problem (crappy juniper srx), fix the problem (replace with mikrotik ccr), job done, drink beer. if (user.exists()) {
destroyHuman(user);
hideRemains(user);
}It's not really a valid reason. Unfortunately, there are still a lot of people who think LOC is a valid metric.
/**
* Get the foo
*
* @return the foo
*/
int getFoo();
/**
* Get the bar
*
* @return the bar
*/
float getBar();
/**
* Set the foo
*
* @param foo the foo
*/
void setFoo(int foo);
/**
* Set the bar
*
* @param bar the bar
*/
void setBar(float bar);
Compare with what any sane project would do: int getFoo();
float getBar();
void setFoo(int foo);
void setBar(float bar);
Multiply that by a dozen such attributes, and what should fit on a single screen spans now hundreds of lines. What you could see at a glance, you have to search for. There's simply no way those utterly redundant comments increase readability. (Yes, the real comments in the real code were just as redundant.)LOC is more valid as a metric than most care to admit.
>LOC is more valid as a metric than most care to admit.
It's not.
I'll repeat my current gut feeling. Intrinsic measures of software quality are terrible as guides for how to write code. They are decent at predicting quality, but are not usable in a prescriptive way to write quality code.
Not all of it, actually.
> nobody had to physically type that
We totally had to read that.
> And usually no developer will be dwelling on this object or this area of the object.
Well… yes, usually. Sometimes however we did want to pay more attention than usual. At that point the original dev would have made a real comment. And that comment was lost in the noise, so you couldn't even distinguish between real comments and useless boilerplate.
int foo; float bar;
That can later be changed, when the need arises.
case class Baz(foo: Int, bar: Float)
Gives you a free hashCode, equals and copy as well.
Maybe I am mentally disabled, but I can only focus on one logical task at a time. Can other programmers multi-task their coding?
I more meant it in the sense that it's nice to be able to soak in more of the code at once and avoid unnecessary scrolling.
The less scrolling, tab management, and other context switching I have to do to look at those "2" functions, the easier it is on my memory.
I don't disagree, but the sentiment seems to be "fit everything on the screen at all times", which seems like one of those rules that may take more time to implement than it saves in actual time.
I work on projects with upwards of 100k lines of code consistently, the very idea that anything relevant will fit on one screen is absurd, so I am always curious what kinds of projects people work on where the relevant code is literally within a half screen of each other.
Given that above description, you can probably guess I don't have everything on screen at all times, not even close. But I'll refactor off small files of related functionality where I can, subdivide things that are getting unwieldy, etc. - put private/static functions near the public/exported functions that call them, break things up into (sub)modules, etc. and get good grouping.
> I don't disagree, but the sentiment seems to be "fit everything on the screen at all times", which seems like one of those rules that may take more time to implement than it saves in actual time.
Taken to the extreme, any such rule absolutely will. I certainly don't have that as a hard and fast rule. Or even as a guideline per se - it's more a faint but distant dream, or perhaps a feverish hallucination, a reminder of codebases past. But there's often plenty of cruft and low hanging fruit that can be fixed up in these codebases. Everyone has their "just one small change" to check in, so things build up and they don't jump to "okay, this is one line too many - time to refactor."
I practice pain driven development - when it's getting to be a pain to keep enough of something my head to do whatever sweeping changes or overhauls I might need to do, see if some pure or near-pure refactoring changes it. No expected behavior changes, often not even class or structural changes - often literally just regrouping code by topic and category via cut and paste. Horrific diffs, but low-risk checkins, and often a lot easier to reason about afterwards, and suddenly related code is often on the same screen.
(Although far from always.)
Yes, it's nice when things are close together, but in my case it's so rare as to not be point to consider trying to achieve, but more like a mental joke like "Hey, Bob. Come here and check this out! Two functions and template I am working on all fit on the same screen!" And Bob's like "wooooah, dude". And then we take a group selfie with my monitor and have a laugh, print out a poster of the selfie for the bulletin board for a few months, then throw it away during a cleanup and move on...
But to have people say "It's important to save whitespace so stuff can all be on the screen to avoid context switching" seems completely and utterly pointless.
void foo(int x) {
if(x == 0) {
bar() }} public class Main {
public static void main(String[] args) {
System.out.println("Hello world!") ;}}
We had a good laugh. (that was fine with him, of course as long as it's a joke)PS: getting back to the original topic, imo, formatting style should be project-wide rules set at the beginning or during refactoring. Don't get too hung up on style as long as it stays consistent.
void foo(int x)
{ if(x == 0)
{ bar(); } }
That is workable. I have adhered to it (mostly) in this Yacc grammar file: http://www.kylheku.com/cgit/txr/tree/parser.yJust in the grammar; not in the supporting functions.
It's an attractive style for C statements embedded in another language, because they look more like capsules somehow.
This is the true sign of a programmer's transcendence. Specifically the irrational joy of seeing net negative LOC diffs.
It's not about how much you can add. It's about how much you can remove without sacrificing correctness, functionality, and readability.
Deleted code on the other hand, is never a liability. The act of deleting it may be, but due diligence should ensure it is not.
Once I was using an industrial camera that had a mode that allowed it to mirror or “flip” the image output. Unfortunately, after running for a few hours, sometimes it would decide to flip the image on its own. Had that “flip” feature not existed, the bug could never have occurred.
The point isn’t that you shouldn’t add useful features, but that even easy to add features aren’t as “free” as they might seem and the risk of adding them should be recognized and weighed against their utility.
He would write helper functions that he thought would be useful before writing actual code and often ended up wasting time. Half the functions he wrote, he never actually used, but he'd still spend time writing them and unit tests for them.
This! Comments should convey programmer's intention and explain the thought process, not the code itself. Also, too many comments usually indicate that code should be refactored.
Exit or get out quick... make it readable... then the meat is left for those that want it below...
Screw that. I've been writing code for 15 years, and Allman style braces make it so much easier to mentally parse code into blocks that they're worth every single LOC. I can't speak for anyone else but I'm not working on an 80x24 terminal anymore.
The correct answer is whatever the project is already using, unless it is new and then you luckily get to choose.
Unsurprisingly, PEP 8 does have a rule for tabs vs. spaces: https://www.python.org/dev/peps/pep-0008/#tabs-or-spaces (spaces, of course)
But tabs vs. spaces is indeed a bad analogy, because of course it's 4 spaces. ;-P
pycodestyle, the Python style checker, has a few checks disabled by default (because there's no consensus they're good ideas), and even some mutually exclusive ones:
https://pycodestyle.readthedocs.io/en/latest/intro.html#erro...
Tabs are semantic and only take one key press for movement back and forth and to delete.
If you see 1 tab you know it meant one indentation level. With spaces you have to think.
Plus with spaces you are stuck with 2/4/8 spacing(unless you reformat), with tabs you can configure your editor to your preferences.
Configure 1 tab to be 4 spaces.
Sometimes it's easier to go with the flow. I like tabs for the reasons you mentioned, but fixed-width spaces are a bit better for some reasons too. IDE's can do the heavy lifting of re-formatting indentation levels and converting tabs to spaces for me, and it means if I cat a file on a remote server regardless of the bash tab width settings or if I'm in your code or the stdlib it will all make sense.
Except that you can't because people using tabs will invariably start mixing tabs and spaces because they can't separate indentation from layout. So the code will be messed up unless you configure your editor to someone else's preferences. Also, I'm sure there is some obscure git setting to de-uglify tab users' diffs, but I'd rather not find out.
Before you say that I'm crazy... please, go and read pep8.
OK. What am I looking for?
Tabs vs. Spaces has a fairly good answer that flows out of this way of looking at things. There is a counter-argument to that answer, but it is invalidated if you move toward conventions for argument lists and chained method calls that are also designed to limit noise in the source control system.
IMO, the argument about bracing also has a practical, non-religious answer once you start looking at your coding conventions this way.
When I write Python I end every block with a "pass" statement so that emacs can auto-indent my code properly. The "pass" statement thus effectively becomes a close-brace. It drives Pythonistas into conniptions, but I never have to worry about reverse-engineering a block of code to figure out how to restore the proper block boundaries after a cut-and-paste has screwed up the indentation.
If boundaries exist they need to be clear. One shouldn't have to count the tabs that make up the level of indentation.
Its already a challenge reading code. Counting invisible tabs makes it even worse.
Similarly, copying code from StackOverflow, Github and random blogs have all worked without any issues.
So it can't be as easy as you imply.
The indentation strategy also looks to produce syntactically valid code, so long as what you are pasting lacked indentation errors.
Mixed spaces and tabs as a next go, and it worked too---converting the pasted tabs into spaces of my style.
Smart-tabbing is pretty smart.
Granted, I knew that so long as the indents were proper (i.e it compiles), that pasting at that point would give valid code but... I can't see how using braces would have been different. Just because it compiles, doesn't mean it works. I've pasted javascript incorrectly multiple time to produce valid, but incorrect for my needs code.
def foo(bar)\
:
if (bar > 5)\
:
return bar + 1;
#
else\
:
return bar - 1;
#
# >>> from __future__ import braces
SyntaxError: not a chanceIt's only people who cut their teeth on adjacent bracing that find it more readable.
I've seen both, and the reduction in vertical whitespace matters more to me than the readability to an untrained individual.
You seem to be erroneously equating a coincidence with a preference.
You should have a pretty cool family!
How big once you remove all the extra newlines?
Thank god OC didn't mention "Just like you move from tabs to spaces."
At the end of the day you can do whatever you want with the code you're writing, and some pre-commit hook can run go fmt/prettier/whatever and then it'll all get standardized.
I've become a very heavy proponent of code formatters (I added prettier to the entire team) and I truly believe that talk about code formatting will never really die off, but at the end of the day you can do whatever you want on your side but still have a standardized looking codebase, and that feels extremely liberating to me.
As a PS: If you are a diehard tab proponent/different bracket placement/whatever but your team formatter configuration uses spaces instead then you can also use code formatters to do spaces -> tabs locally, say, on file open. Once you commit your file your precommit falls back to the team configuration, which removes your tabs. Everyone is happy.
I should see if he remembers where he got it.
So totally this. I despise indenting with tabs with a passion, but when I heard gofmt did that I was actually pleased. There's no arguing with the official formatting utility: https://golang.org/doc/effective_go.html#formatting
I just defer to the code formatter. That way everything is consistent looking internally, which is all I care about. I couldn't care less whether the braces are on the same or a different line or other things like that outside of the fact that I do like internal consistency within projects.
All that aside, as soon as I saw the OP title I knew (via years of exposure to bikeshedding programmers) that this thread would end up having a ton of comments on it and wasn't surprised to see it was up to 500-something and still rapidly climbing.
Same goes to spaces vs braces, editor wars, etc.
Downside: my projects are usually a bit of a mess with mixed indentation styles :D
To me, it looks weird when functions don't have that extra line of space to set off their definition, but if/while/whatever aren't special enough to need that call out.
That said, the most important factor is simple consistency. In my person projects, I have the hybrid style. At work, I use same-line-only. If I'm on a project that has next-line, we all use next-line.
int max(a, b)
double a, b;
{
return a > b ? a : b;
}PS: he coded at Hercules the graphic cards back in the day before he got into teaching.
As everyone else seems to use K&R I've just had a adapt over time :-(
I like One True Brace Style (1TBS) with an uncuddled 'else', which has else/else if on new-line with the break before it (similar to Stroustrup K&R without any same line properties, one thing per line, no bracket-less statements) and setup the standard that way if I am designing it, but do Allman or whatever variant the codebase uses if it already exists.
There are lots of sane ways to read code, but consistency is key. It's hard to change muscle memory in how I type, though, so I think auto format is the way to go.
https://softwareengineering.stackexchange.com/questions/9954...
The name tells you that it won.
A later comment on the SO post described how K&R can have maintenance costs because when moving things around it's more difficult to tell where a statement ends, and can accidentally cause side effects.
The only thing being considered was "bugs in the final code", not "time and effort in maintenance."
To say it is religion isn't to say "your preference is dumb". In my own code preferences I try to maintain a split between "have a reason I have confidence in" and "personal preference". And "easier to read" is almost always in the latter. (If you can say WHY, that is the actual reason, but your actual reason still has to be provable.)
Given the difficulty of finding good research and the inherent difficulty of the research (if familiarity is a big component to preference, and a proven ability to mentally parse code required to judge, then good luck running a control group) I have a lot of items I have confidence in being provable without having actual proof, but the lengthy preference list also means I'm willing to accept that each of those can switch columns.
Way too many of our "easier to read" defenses are really "it is easier to read because I'm familiar with it. I personally think camelCase is terrible - language has spaces for reasons! - but there is no denying that it is very common and that the vast majority of coders learned it first, an unfortunate self perpetuating cycle.
Nonetheless, shown research that said I was wrong (assuming said research had taken familiarity into account) I would change my stance rather than dig in my heels...even if that change were to only add "but that's just me" to the end of it.
The advent of automatic code formatting (and Python pioneered this in many ways with pep8, but even huge C++ projects are realizing the value in clang-format, clazy, etc) put the nail in the coffin of arguments against whitespace blocking. You can have an enforced uniform style throughout a project now, with no ambiguity, and in such contexts using whitespace as a block delimiter also has no ambiguity. It just takes advantage of formatting already being done to avoid redundant glyph use.
Yup, Python clearly pioneered automatic code formatting. Nobody had done it before them...
That document states Bill Gosper wrote one of the first pretty printers, but doesn’t give a date for it.
I would think it is much older, as writing a simple lisp pretty-printer is easy and reading lisp without autoformatting is “less than ideal”.
In what way are context-free languages "a grammatically bloated mess?" Whitespace delimited languages like Python have context-sensitive grammars. Now that is a mess.
Dismissing the whole field of formal language theory with that statement is so bogus, it "is not even wrong." There are tools and techniques for parsing context-free languages that are impossible with context-sensitive languages. Parser generators, structured editing, metacompilers, composable grammars; those are all impossible with context-sensitive languages. Context-free languages are faster to parse, easier to write compilers/interpreters for, and much easier to write tooling for (editors, linters, etc).
Ambiguous context-free grammars are a very different thing from context-sensitive grammars. You can't point to the former and say "so whitespace sensitivity is ok."
I can point to ambiguous context-free grammars and say whitespace sensitivity is ok, because the tools to fix the one are often the same to fix the other. (Ambiguous context-free grammars do turn out to make context sensitive languages, because it's a blurry line in the grass between them.)
Whitespace sensitivity is one of the easier context sensitivity challenges to embed as a sub-language in an otherwise (mostly) context-free language, because you can represent it entirely as pseudo tokens from a simply modified lexing phase in a traditional CFG parser. Python is an easy and clear proof, as that is exactly what it did (Python is also not purely a CFG after whitespace tokenization because of also how it handles the dangling else, but we've already mentioned those weeds).
Even if you don't find the boundaries between the classes of languages fuzzy (and Parser Combinators and PEG grammars have a lot to say here), context-sensitive languages are a tool in a growing toolbelt, not a "complex" evil to be demonized.
Algorithmic complexity is not "moral superiority." The whole point of formal language theory is to show how the different languages differ in terms of how difficult they are to work with.
> just as they use regular expressions for tokenizing and allow regular language sub-languages
That is because regular languages are a subset of context-free languages. There is nothing special to "allow" there.
> I can point to ambiguous context-free grammars and say whitespace sensitivity is ok, because the tools to fix the one are often the same to fix the other.
No they are not. Precedence rules and extra lookahead will help with ambiguous context-free grammars but not with context-sensitive ones. There is also a whole class of algorithms, such as GLR, specialized to handle ambiguous CFGs efficiently.
> Whitespace sensitivity is one of the easier context sensitivity challenges to embed as a sub-language in an otherwise (mostly) context-free language, because you can represent it entirely as pseudo tokens from a simply modified lexing phase in a traditional CFG parser.
That statement is nonsense. There is no way to "embed" a context-sensitive language in a CFG. What you are saying is that it is "easy" to make a whole context-sensitive parser just to produce a context-free language so you can pass that to another CFG parser. That is not "easy," that is bolting crap onto other crap.
Chomsky's hierarchy was designed for production from categories of languages. That it doubles as a useful rule of thumb for algorithmic complexity in parsing is a fascinating dualism in mathematics. That those same rules of thumb reflect basic automaton abstractions is even more fascinating. This is also where it seems the clearest analogy lies to why I find your arguments to "complexity" so useless. It sounds to me like a strange steampunk form of the "640k is all anyone will ever need" fallacy: "why use Turing machines when Pushdown automatons will do just fine?"
Yes, there are a lot of great tools for working with CFGs, just as we've pushed Regular Expressions far past the boundaries of formal Regular Languages, we push these same tools past the boundaries of formal CFGs. We aren't actually constrained by the limits of only using Deterministic Finite Automata or Pushdown Automata in our programming, we have vast Church-Turing–level state machines at our power completely and easily capable of tracking contexts/states.
The "context" in a whitespace-sensitive language is "how many spaces have you seen recently?". This is not a hard question, this is not some mystical "context sensitivity" that needs an entire "context-sensitive language parser". It's a handful of counters. That is the very definition of easy, that's the built-in basics of your average Church-Turing machine, keep a number on the tape and update it as necessary.
Python's specification for the language defines the language in a CFG BNF just as the majority of brackets languages. The one concession to its whitespace sensitivity is that it expects from its lexer INDENT and DEDENT tokens. Those tokens are added simply by counting whitespace between lines, seeing if there is a < or > relationship. After those tokens are in the stream (along with the rest of the tokens defined in their associated (Mostly) Regular Languages) it is parsed by whatever CFG automaton/algorithm the Python team wants (LR, GLR, LALR, PEG, etc). That's not "bolting crap onto other crap" by most stretches of the imagination, that's using a pretty simple stack of tools that any programmer should be able to (re-)build, and not one at all more complicated than (and in fact built entirely inside) the classic tokenizer/lexer feeding a parser stack.
Honestly, from everything you have posted so far, I really get the impression that you have never worked on programming language parsing, or with large code bases. The difference in algorithmic complexity is not "useless," it is the difference between being able to compile millions of lines of code in a few seconds on modest hardware, and needing a cluster just to get your builds done:
https://community.embarcadero.com/blogs/entry/compiling-a-mi...
If we want to fight anecdote for anecdote, I've certainly seen millions of lines of code of Python analyzed ("compiled") in seconds on modest hardware, rather than a cluster. Differences there are more in the static typing versus dynamic typing, and strongly compiled versus weakly compiled/primarily interpreted there. Maybe a better anecdote is millions of lines of Haskell? Seen that too. Whitespace-sensitive languages scale just fine.
I can understand it if you want to admit that you just don't like whitespace-sensitive languages for personal reasons, but there aren't very good technical reasons and you seem unhappy with my explanations of why that is the case. I'm not sure how much more I can try to explain it, and given you seem close to resorting to personal attacks I'm afraid this is likely where the conversation ends.
void foo() {
code...;
}
void foo()
{
code...;
} stuffstuffstuff....
stuffstuffstuff...
So I read C and Python (and Lisp) code the same way. A naked open brace looks jarring and ugly to me. It also increases the separation of other related parts of the code, i.e. if (...) {
while(...) {
do(...) {
vs if (...)
{
while(...) {
{
do(...) {
The latter seems unnecessarily wasteful to me. void MyLongMethodName(SomeLongParamType param1, SomeOtherLongParamType param2,
YetAnotherLongParamType param3) {
if (longContrivedVariableName1 == longContrivedVariableName2 &&
longContrivedVariableName1 != longContrivedVariableName3) {
// do stuff
}
}
void MyLongMethodName(SomeLongParamType param1, SomeOtherLongParamType param2,
YetAnotherLongParamType param3)
{
if (longContrivedVariableName1 == longContrivedVariableName2 &&
longContrivedVariableName1 != longContrivedVariableName3)
{
// do stuff
}
} void MyLongMethodName(SomeLongParamType param1, SomeOtherLongParamType param2,
YetAnotherLongParamType param3) {
if (longContrivedVariableName1 == longContrivedVariableName2 &&
longContrivedVariableName1 != longContrivedVariableName3) {
// do stuff
}
} void MyLongMethodName(
SomeLongParamType param1,
SomeOtherLongParamType param2,
YetAnotherLongParamType param3
) {
if (
longContrivedVariableName1 == longContrivedVariableName2 &&
longContrivedVariableName1 != longContrivedVariableName3
) {
// do stuff
}
}
I also make liberal use of temporary variables: void MyLongMethodName(
SomeLongParamType param1,
SomeOtherLongParamType param2,
YetAnotherLongParamType param3
) {
auto condition1 = longContrivedVariableName1 == longContrivedVariableName2;
auto condition2 = longContrivedVariableName1 != longContrivedVariableName3;
if (condition1 && condition2) {
// do stuff
}
}
Vertical space is allocated to the actual parameters and conditions, and the extra syntax-only lines exist only to separate chunks. This works well with "paragraphs": void MyLongMethodName(
SomeLongParamType param1,
SomeOtherLongParamType param2,
YetAnotherLongParamType param3
) {
auto condition1 = longContrivedVariableName1 == longContrivedVariableName2;
auto condition2 = longContrivedVariableName1 != longContrivedVariableName3;
if (condition1 && condition2) {
// do stuff
}
auto someData = buildSomeData();
doSomethingWith(someData);
while (someCondition) {
// loop
}
finishUp();
return someData;
}Allman is great for this. Though I'm a Whitesmiths guy, myself. Still, same idea.
/36 years of commercial coding here.
Even with 80x24 (don't ask), I fully agree with Allman style braces.
Unless you are writing completely brilliant code your future self will hate you if you skimp on comments. Also self-documenting code is great, but intentions are not always clear to another person trying to figure out your code.
Documentation explains HOW to use a thing. Good comments explain WHY a thing is strange. Bad comments explain WHAT a thing does and must be made redundant by extracting and naming the thing.
Probably overly generalized to be pithy, but no.
Comments are by very definition documentation, which can and should cover of all of the what/why/who/how/where.
Documentation that occasionally explains how to use code can actually be useful!
Unless you actually get around to writing a user manual (which gets out of date), please do consider documenting how something should be used, particularly in libraries.
Also, i never said documentation isn't useful. HOW and WHY are useful. WHAT in documentation and comments isn't because it should be in the variable/function/method/class/instance name.
(Who/Where/When are covered by the source repository and the blame function.)
Plain code organized into understandable methods (usually no more than half a page of code), with good naming for variables & method names reduces the need for comments.
It's also easier to scan/read code if there's a minimum of comments in the way.
Clear, self-documenting code is great but it can't capture that holistic insight into what the whole program is doing. It can't capture context, because the whole point of clean refactored code is to be as context-free as possible.
As an example, our code base has a comment which refers to our internal issue tracker which itself refers to this HN comment: https://news.ycombinator.com/item?id=9048947
I was secretly hoping you actually meant your own comment, creating a recursive loop. But a surprise John Nagle is even better
Or sometimes, not even that: issues elsewhere in the system, possibly completely outside of your control (e.g. the language/framework/library you're using, the OS, business requirements, etc).
I look back at a lot of my old code and even without comments, it's easy to follow the logic because I used sensible names and constructs.
Often it means you can refactor it so that there is only one of any "thing" so there is no need to clarify which version or role this "fooBarThing" is serving to disambiguate it from the other "bazBarThing".
Functional composition and well structured data is the key to this. It's basically halfway to point-free style.
Readable variable names doesn't mean wordy names. I would avoid using more than 2 words in a variable name.
You're lucky if you're dealing with code that's either small enough or simple enough not to need the added context...or if you only need to read your old code.
Code shows what is being done. Comments should explain why it's being done.
Often you have horrible complexity imposed on you from outside -- business rules you're implementing, etc, which are _not simple_. You can write code that encapsulates a lot of that, and even refactor it so you can see _what_ the code is doing pretty easily ... but it's often _very_ valuable to document in a code comment (docstring, JSdoc, etc) WHY it's like that.
Arguably, the comment is not to explain the code, in this case, but rather to explain the twisted bureaucratic logic you're having to implement, so maybe that still counts as "brilliant" code. ;)
Moreover, when the weight of those comments trends towards 50% being justifications for why you did it that way, it's an excellent sign that your code has gone from "brilliant" to "super-genius", as in "Wile E. Coyote, Super-Genius".
"Completely dumb" code can be read by your future self without comments. Even, sometimes, by other people!
But for the general case, I do wholeheartedly agree with you.
That's like saying "the problem with healthcare is that it costs money". Of course comments can get out of date. The solution isn't to throw them out!
There's alternative to in-code comments answering the question "why" - it's commit messages. They can't be out of date.
git log -L0,10:file.txt
Write good commit messages and comments that answer "why" are redundant too. Commit messages by their nature refer to the exact code that they refered to when they were written. Comments answering "why" after a few years are misleading anyway, because code changed around them.
Commenting public api etc is obvious, and most people do it.
If there's not 1 line of code from that commit remaining why is it relevant?
If something looks weird still - I go back to the commit that created this part of code and git blame that. I don't remember a case where I had to do 2 steps like that.
BTW we have a rule of putting JIRA ticket numbers in commits, that makes it even easier to find out. You can see the whole discussion that resulted in the code you try to understand, test cases that you don't want to break with your new changes, etc.
sounds great until there's a refactor (including moving code around), then all the "comments" get buried.
So something entirely outside the codebase, which may or may not be available to the person who needs the information, and which may or may not need to be extensively searched through years of history to find the relevant commit message, which may or may not be sufficient to explain the code... is better than having a comment in the code.
Likewise, your future self will hate you when comments fall out of sync with the code.
If the code is readable you don't need much in the way of comments.
That should leave you with a solid documentation of your application's components and APIs and a small amount of inline comments.
If you find you have more comments, either your code should be simplified or you're commenting trivialities.
As everything, it's a guideline with exceptions, not an absolute rule.
The vast majority of comments I usually see can be rolled into variable names or function names. If you're writing lots of comments, that's a good indication your code is hard to understand, your variables are badly named and your functions are too long in my opinion. I think people that say "your code is bad if you don't have any comments" have things backwards personally.
Pretty much the only time I use comments is when I'm forced to write weird code to workaround an API bug, to explain an unintuitive optimisation or to give high-level architecture documentation.
I've honestly returned to code I've written myself maybe 5 years later and rarely had an issue that would have been helped with more comments.
If it was this simple we wouldn't be still arguing about coding styles decades after they were invented. I know few old programmers that I respect very much, and they don't agree about the perfect coding style, not even simple stuff, like braces in separate lines or not.
Money quote.
I disagree with the logic in this sentence. New coding styles are invented all the time. The guard statements in the article were only formalized in the late 90s, for example. These arguments about coding styles "decades after they were invented" are the only way we know which work and which don't.
It's not a matter of taste. It's how you separate the wheat from the chaff. Except for the brackets one. ;)
" Best thing I learned about commenting is: Comment WHY you are doing it... not WHAT. Code is the WHAT... Comment is the WHY
# open file
with open(my_file, "rb") as binary_file:
binary_file.seek(8)
No mention of what was at byte 8, or what the hell it was doing, but he commented the only obvious part of the code.I had the same thing happen about 10 years in. I just got sick of the extra coding and long indented blocks. And just switched over one day.
Why is this not taught from day one?
There are times where early return makes a ton of sense. Usually that's the case when you can derive the return value from a shallow interpretation of some input values (i.e. check preconditions) but still require more complex processing for other input values. In that case, return early, but constrain the rest of the method to one return point.
On the flip-side, there are cases where an early return makes it incredibly hard to reason about the function. This usually occurs in dense, nested conditional logic. In those cases, I found sticking to rule that a return value should be initialized once and only once, and return from a single point, beneficial in even reasoning about the problem.
But as a novice youd should adhere to rules. As you gain experience you learn when to break them.
I think the "rule" should be "exit early", and the exception "multi-indent exit later".
Having said that, early return should be for error/ignore situations - there should only be one return for a result and that should be at the very end of the function.
I now think of the early part of a function as a filter for bad params, invalid state & whatnot, and return as early as possible.
Code just feels less complex that way.
I agree with all your other points and I‘d even agree if you wrote return early makes code more readable but not that it‘s something experienced programmers do.
Programming is still - to some degree - resource management. Inexperienced devs often miss that fact, because they are focused on memory management and believe they can rely on the garbage collector for clean-up. In real projects you quickly end up managing file descriptors, sockets, handles, GUI resources, etc. Early return is terrible for that, because your coding style starts to depend on the kind of resource to manage.
That‘s why the more experienced folks end up writing single exit point code sooner or later.
EDIT: I‘m not arguing that early return and proper resource management are incompatible, just that most experienced programmers have often been burnt enought to avoid it.
Yes, RAII works for the casual std::vector, but it's not a maintainable solution for general resources.
EDIT: and I was the lead for a high availability C++ RTOS. I know that not all patterns fit for writing quality, available code. This fits remarkably well though. Even though we were totally async and didn't rely on the stack for context lifetimes.
The guard object is there (e.g. "on the stack") but the destructor is not defined in-line but in a separate class method definition. Classes have always that implicit ideal of being "isolated". This is wishful thinking, of course. In the end all the parts of the code need to contribute to the program. That's why OOP codebases often end up as a terrible mess. Classes can't decide if they want to be isolated or involved. The compromise is all this terrible implicit state. (do you know the quote by Joe Armstrong about the banana, monkey and jungle?)
The resource almost all programmers are managing is the resource of human time. Human time to code the project, human time to review the code, human time to alter the code after the fact, human time to port the code to different environments. If you're writing code to run AI at Google then fine, you're managing resources at a level where performance starts to hit limits that warrant a style change, but generally speaking you know that ahead of time and you choose tools to handle a project of the right scope, but generally speaking exiting early isn't impacting your code performance. Big O mistakes are.
Also, some of your examples (like altering or reviewing code) can become easier and faster if resource management is a first-class citizen in your code base or language (i.e. is either more explicit in the code, and you know to look for it and manage it, or it is somehow taken care of automatically by your language, so you would have a hard time forgetting it, even if it is handled implicitly).
And I'm not saying that finally depends on checked exceptions, I'm saying that pattern is very brittle. You change lower code to throw a new exception, and you change the above code to catch it like you're supposed to, but now the middle code has no idea that there's this new exception and leaks resources. So you end up either having brittle code who's correctness depends on implicit choices of the code around it, or you're wrapping pretty much all function bodies with try-catch-finally-rethrow.
If you’re able to follow that pattern then safe handling of unusual control flows tends to follow naturally. You just adopt whatever mechanism your language provides for scheduling the code to clean up and release resources at the same time as you allocate those resources. Almost all mainstream languages at a higher level than C provide a suitable mechanism today: RAII, some variation of `with` or `using` block, try-finally, Lispy macros, Haskelly monad infrastructure, etc.
That way, the only places you need to write any extra logic are the layers where you allocate/deallocate and possibly lower layers that manage side effects on those resources in the unusual case that they require a specific form of recovery beyond the default behaviour in the event of aborting early.
Hopefully if you do have to work at low level like C or assembly language and you’re using forms of control flow that could bypass clean-up logic then you already have at least some coding standards established for how to manage resources safely as well.
You misunderstand how `finally` works. The whole point is that it runs regardless of exception type. So the resulting code in the middle layer doesn’t need to know anything about the code it calls, and doesn’t leak resources, even when calling or called code changes: It’s not brittle.
> or you're wrapping pretty much all function bodies with try-catch-finally-rethrow.
No need for `catch` and rethrowing. And in the case of C# and modern Java, no need for the rest either: You only need to wrap resources that you allocate into `using` (C#) or `try` (Java; but not `try…finally`! [1]). Sure, it’s more verbose than RAII in other languages with automatic storage duration (like C++). But it effectively performs the same.
[1] https://docs.oracle.com/javase/tutorial/essential/exceptions...
Exactly, simple but not too simple. If the condition does not need to return early or can't, you don't.
> quickly end up managing file descriptors, sockets, handles, GUI resources
In some of the cases you describe you wouldn't return early or depending on the need, but if you can you should to minimize work.
Some examples: file descriptors you'd return early if the file does not exist, sockets you'd return if dropped or compromised and you have performed cleanup, handles if null, gui resources can be lots of things but if you were doing something where you needed to cleanup you can cleanup and return. In C# for instance you might wrap some of that in a using such as files, sockets, streams etc. Most of your examples you might already be in the meat of the return early flow and usually there is still a return at the very end but in/after the meat.
Returning early isn't saying do a dirty break or asserting out, you don't return early before cleaning up if you are within some resource.
Even C has auto cleanup with __cleanup__ attribute in both gcc and clang.
Other languages have constructs that obviates this pattern, for example Python has `with` blocks that acquire resources and free them upon leaving the block, and Go has a `defer` statement which adds code to some LIFO structure that always gets executed upon returning.
> Same way braces go on the end of the method/class name to reduce LOC.
For me, these are exact opposites; the reason to not use one-liners is the same as the reason to put braces in separate line: both improve readability.
Same way you learn to let some code fail instead of handling every exception.
indeed I've evolved it myself but always wondering if it was the right thing to do. Now I even have a name for it.
This is one of those things that whether you drop it or not, you recognize the times it's caused you severe pain because you didn't notice it was single line when adding a statement, such as a debugging one, and all of a sudden the conditional part isn't what you were expecting.
This is actually one of the things I love about Perl. Single line conditionals were changed so they both looked different, and they only allow a single statement. e.g.
if ( condition ) statement;
becomes statement if condition;
Regular if blocks still work as expected, but there is no braceless version of regular if conditions. I understand many people find it jarring at first, but it does allow for clear and concise precondition and return early statements, especially when grouped together. e.g. return undef if $param1 < 0;
return undef if $param1 > 100;
die "Not a number" if not looks_like_number($param1); if (condition){
doSomething()
doSomethingElse()
}
else
doSomethingDifferent()
doSomethingConfusing() // is this part of the else or not??
Perl got many things right. Its a shame it seems to have fallen away as a language.That's not true.
Sometimes, the code begs for an early return so you can focus on the meat of the method. Most times, that's not the case. Single return conditions should be the default, and you should have a good reason not to do the default. In Java, for example, if you also stick a 'final' on the uninitialized variable, you have very nice compiler check to enforce that every code-path initializes the variable once and only once. Sometimes that is also too strict for what you want to do, so you can break the rule, but you have to have a good reason.
>Same way braces go on the end of the method/class name to reduce LOC.
No. Just no. Not for that reason.
>Same way you move on from single condition bracket-less ifs. (debatable but more merge friendly and OP hasn't yet)
It's only debatable by people who are just used to it, or who want to minimize LOC for insane reasons. It's non-debatable in that it is a very common vector for bugs to sneak in. Over the last 5 years, there were probably as many bugs stemming from bracket-less ifs in our codebase.
>Same way you move on from heavy OO to dicts/lists.
You start introducing another common vector of bugs for a little flexibility. Sometimes that may be worth it. As a general rule, it's not worth it.
I generaly don't like when one person says (declares) how experienced persons act, but in this case I have to confirm with myself, as I also "evolved" to it and I tend to program in isolation, so I am no influenced by others so much.
(This example comes from Elixir.)
value = if boolean, do: this_value, else: other_value
is undefined in some circumstances unless an "else" is included.
This pattern might seem weird to OOP folks (the assignment, not the ternary-operator-style logic), but in Elixir-land, it is not only considered superior to assigning inside if/then logic, you actually get a compiler warning if you assign/bind to a variable inside a logical expression because (again) that value becomes undefined as soon as you leave local scope (or re-acquires the value it had in the outer scope, thanks to immutability).
And if you think about it, values can only travel in one direction using this scheme, which further simplifies reasoning about information flows.
I would argue that this is also superior to early-exit as having one entry point and one exit point is much easier to reason about, and because the extra logical complexity you're ostensibly trying to avoid writing by early-exiting is actually still there, it's just obfuscated... and that complexity should be made obvious (in which case, if it's extensive, it would be a code smell... consider the example of a function with 25 early exits that "looks flat" visually, but which actually has 25 different branches)
value = boolean ? this_value : other_value;
If the branching logic gets too complicated, I usually move it into a function (private method) with a return in each branch. let value = do {if (boolean) { this_value } else { other_value }}
...which is even closer to the Elixir style than JS's ternary operator (which is identical to Java's).Is that a thing? No question that many concepts of OOP are in heavy need of reform, but I didn't know the basic idea of a struct was one of them.
I can sort of imagine this for quick-and-dirty things in untyped languages - but if you have types, passing dicts around everywhere seems like needlessly throwing away type safety - while it's also cumbersome to code and less efficient at runtime.
Even if I'm getting from a DB which won't be changing out from under me, many times I'd prefer just getting a simple Map rather than spending time creating a "safe" object. Do I create a User and UserWithDetails? Or always return UserWithDetails but sometimes fields are empty because I don't care about them? Dicts are light, objects are cumbersome.
I also see getting parse errors when APIs change as a positive thing. When you change an API, things do break. Learning what breaks as early as possible is good. It's not even about objects vs. dictionaries, you could use any structured composite datatype and the overhead you associate with it depends very much on the language.
Of course, for a quick hack I'd happily dump data into a dictionary and call it a day. But if I want to learn why it breaks 5 years later, having a clear specification of the expected data is much better even if it cost me an extra few hours.
So you end up with the best of both worlds (in my opinion), where your state is made up of simple plain objects and you behaviors are just functions that accept simple plain objects, but you get all of the benefits of compile time static type checking because they are checked against the interface.
interface SomethingWithAge{ age(); }
class Person{ age(); // age in years }
function classify( SomethingWithAge item ){ if( item.age() < 20 ){ print( 'young') } else{ print( 'old' ); } }
p = new Person( 33 ); classify( p ) // prints 'old'
class Message{ age(); // age in milliseconds }
c = new Message( 231443 );
classify( c ); // prints 'old'
It's in the roadmap, but discussions about it are still going on four years after the ticket was opened...
My understanding is, by “heavy OO” they mean custom container-like classes implemented by encapsulating a list or dictionary, but exposing non-standard APIs for get/set/erase/replace.
By the OO books that’s the way to go, because object state encapsulation. Practically, exposing raw lists/dictionaries/vectors/iterators at the class API boundary is more flexible because it makes the collection compatible with lots of other code, both in standard runtime and third-party libraries.
Another thing, if the only state that container holds is the collection of items, maybe you don’t need any container class at all. Just write global functions / static class / static methods that directly process lists or dictionaries of these items.
OO can run into versioning issues with fields/structures changing and needing to have many versions of those over app iterations just dicts/lists. Database fields being added and removed and supporting old versions is more easily solved in basic dicts/lists that map to json/xml and the base of any language. Dicts/lists are very interchangeable across systems and are the base of API outputs and the ease of use in javascript, python and other dynamic uses.
There should be a reason you are using structured OO but many times it becomes as weight, though there are good times to use it and most projects have some of it. Good reasons are hardened/non-changing codebases, possibly native apis/libs to help understanding, but when it comes to rest/http apis usually you are building OO that consumes data simply to return to dicts/lists when output/input into the apis/client-side/etc.
Classes/OO/types can be built as well with objects that are based on dicts/lists that do have some internals and helper methods that get/set keys and values and perform actions if you don't want to do that with another context class/api, or wrap dict/lists so that the serialization/deserialization and versioning issues are not a problem as the flexibility of dicts/lists and the benefits that brings in simplicity are still there but there is more structure where needed.
Except that I see soooooo many people try to write state machine code with early return/function encapsulation just to avoid indentation.
And then they get pounded when the state machine needs to evolve--and it always needs to evolve.
I really wish programmers had to design a VLSI chip, get it manufactured and debug it just once before they get out of school. The "So THAT'S why they have a global clock that synchronizes all the parallel operations in lock step" moment tends to be blinding.
What does that mean?
one thing i do disagree with is commenting, there is a time in a place and its hard to derive on a rule on exactly where, when and how, but well placed comments are VERY important and can save developers a ton of time.
I used to hate being forced to use them, just because as a recent computer science graduate I was happy with nested conditionals everywhere. But guard clauses make life so much easier, I eventually discovered.
They make testing your code a lot easier, and also make reasoning about the code and reading it so much easier.
Ruby makes writing methods with guard clauses a breeze, since you can just write something like `return if x > 7`.
e.g.
const check(cond) => cond && otherValue
If `cond` is falsey, it returns `cond`.This means the function could return any of: `false`, `undefined`, `null`, '', `0` or `NaN`.
This is a fairly common issue I see with React + JSX:
<div>{cond && <SomeComponent />}</div>
If `cond` is `false`, `null`, `undefined` or '', everything will be just fine, but if `cond` happens to be a zero or `NaN`, suddenly you have a weird `0` or `NaN` rendering in your page. Oops.Low cost, much safer guard:
const check(cond) => !!cond && otherValueThat's the problem with antipatterns. It's impossible to remain alert to the edge cases, and then they eventually bite you in production. Better to lint them away. But it's difficult to convince people not to do something really convenient if they haven't gotten bitten yet.
cond? && otherValueSame behavior on Ruby:
def content_markup(children)
case children
when String, Numeric
children
when NilClass, TrueClass, FalseClass
return
else
# ...
end
end
content_markup 'foo'
#=> "foo"
content_markup 0
#=> 0
content_markup true
#=> nil
content_markup false
#=> nil
content_markup nil
#=> nilIf you start thinking it is a "guard operator", I would think that going down the wrong path, which might promote misunderstanding of the behavior.
This isn't a misunderstanding, binary logical operators in JS short-circuit like this by design. I believe && and || returned a boolean value in the past, but were explicitly changed to support this behaviour.
The production LogicalANDExpression : LogicalANDExpression && BitwiseORExpression is evaluated as follows:
1. Evaluate LogicalANDExpression.
2. Call GetValue(Result(1)).
3. Call ToBoolean(Result(2)).
4. If Result(3) is false, return Result(2).
5. Evaluate BitwiseORExpression.
6. Call GetValue((Result(5)).
7. Return Result(6).
That is, the behavior has always been "If the first expression is false-ish, return the first expression, otherwise return the second expression" ("BitwiseORExpression" is a class of expressions that include a lot of things, including equality operators).JavaScript does not have any operators that is not explicitly listed in a version of ECMA-262. It would be correct to refer to the construct as a "guard", but incorrect to refer to it as a "guard operator". Calling it "guard operator" also does not promote an understanding of the underlying construct.
1: https://www.ecma-international.org/publications/files/ECMA-S...
The ECMA spec only defines Javascript 1.3 and above. See this description for logical operators for Javascript 1.1: https://web.archive.org/web/20060318153542/wp.netscape.com/e...
Doing some research, it appears that Netscape changed the logical operator behavior in version 1.2 (https://web.archive.org/web/19981202065738if_/http://develop...).
However, they did not highlight this change at all (https://web.archive.org/web/19970630092641fw_/http://develop...), so I suspect it was just a minor cleanup, potentially related to the equality operator change.
As a side-note: Netscape scripting was awful. This was how you casted an object to a Number:
Number(x) = x;I work in Python, and one thing I do if I end up needing to use the same guard clause in several functions is to turn it into a decorator. That way, if I need to update the logic, I can do it in one place and have it propagate everywhere and it makes the function's dependencies clear at first glance.
This was my introduction to aspect-oriented programming, which I've thoroughly fallen in love with.
Are there any preprocessors or libraries to make statements like this easier in Javascript? Sometimes the guard statements can be longer than the function code and I go back and forth on whether to throw (forces the caller to anticipate) or return (causes lots of silent failures).
It would be great to use a defined approach.
This article also discusses the solutions (there's a lot of overlap with OP's article).
If the conditions are semantically symmetrical, if/else is the right approach:
fun max(a, b) {
if a > b { a } else { b }
}
But is it a precondition which causes you to skip the primary logic of the function, then get it out of the way early: fun max(a, b) {
// special case for null
if a==null or b==null { null }
if a > b { a } else { b }
}
Yeah we want to reduce indentation, but not at the cost of making the code overall harder to follow. The logic of "max" is much clearly expressed as a single expression with `else`.Early returns are not a panacea. If/Else-everything is not a silver bullet either.
fun max(a, b) {
if a > b {
return a
}
return b
}
Not saying this is necessarily better or worse. The point I want to make is that your case isn’t special. if (x > y) {
return x;
} else {
return y;
}
if (x > y) {
return x;
}
return y;
return (x > y) ? x : y;
The logic would be convoluted if you are going through extra hoops in order to write your logic like this, making the flow of the application unclear. For some things, an else branch ends up just being easier to deal with. This is especially true in languages like Rust, where if/else is an expression, leading to the following construct being used often (although usually with more complicated content): let max = if (x > y) {
x
} else {
y
};
Note that "max" is immutable despite the conditional assignment due to the if being used as an expression.If it's a single line of return in both branches, then a ternary expression is usually going to be ideal instead.
I do not agree with the conclusions you draw at all, but it is hard to continue the discussion without examples. There are definitely cases where an else clause is the natural choice, but I believe that these are in the minority.
But that max function should have an else statement since it's part of the logic. Less code doesn't always make it more concise. If there's a more complicated logic, then it'd actually be harder to understand at a glance.
Nothing about early return indicates a special case, but rather just indicate that a conclusion has been reached. A few examples:
1. A function that compares two arrays, and first checks if they are null or if their lengths differ before checking their individual elements, and potentially recursing. The early returns are likely to be the hottest section of the function, with the element checking being the special case.
2. A function that searches a list or tree for a node that matches a set of conditions. All but at most one run will use the early returns, making the corpus of the function the special case.
3. A function that does some processing, with fast paths that handle the vast majority of data, but a slow path for when the fast paths do not apply. The fast path is an early return, but the slow path is the special case.
In other words, I believe that it is incorrect to consider an early return to be a special-case, and interpreting code like so might result in misunderstandings. You should look at the condition to see if it is a special case. The only thing an early return indicate is that the return value has been decided, and no further processing is needed.I still think that all my examples communicate the exact same to the reader.
foo(things) {
stuff = do_work(things);
if (stuff) {
return a;
} else {
return b;
}
}
into foo(things) {
stuff = do_work(things);
if (stuff) {
return a;
}
return b;
}
in code reviews.Ok, I really wonder if in this special case the logic just looks _odd_ because of bracing styles. (bear with me)
This is very easy to understand, where as the parent example, not as much.
fun max(a, b)
{
if a > b
{
return a
}
return b
}A max function is one of those rare moments where a ternary statement just seems right, but it's an admittedly simple example.
I think this is interesting. I wonder if our personal experiences with how we learned programming, maybe first languages or first teachers or jobs, etc... would effect our perceptions on logic flow.
I simple don't see anything unusual about the logic flow in the example. It's a simple if/else - return with less cruft to me.
The control flow isn't confusing either way, and some language linters will insist against if/else in this example, but I see the if/else approach with slightly more clarity. In general, though, I'm a big proponent of early return.
But if you are using a more imperative language where “if” is a statement, I think the merits of early return vs. use of an else clause are more balanced in this case.
return (a>=b) ? a : b
is probably the best option in those languages. The level of complexity at which the C ternary operator ceases to be ideal, I think, is lower than that of expression-if.edit: now that I've looked through the comments I see these are mentioned on numerous other threads.
[0]: https://en.wikipedia.org/wiki/Guard_%28computer_science%29
Writing clean and maintainable code is as art and requires balancing many variables. Sometimes an early return is warranted, sometimes an if/else might make more sense. If you're writing C code and you need to release some resource before returning (mutex, file handle etc...) then an early return might be error-prone and an "else" or "goto" might result in cleaner and more readable code. I use gotos a lot in error handling in C, I think it's a superior pattern to the one proposed by TFA if you have a lot of cleanup to do. In C++ and other object-oriented languages it's less of an issue thanks to RAII and/or GC.
Beyond that if/else might be more readable if both clauses are on the same "level" semantically. For instance if you write:
if (some_cond) {
do_a();
return;
}
do_b();
It looks like `do_a` and `do_b` are not on the same "level", semantically speaking. If instead they're two variations of the same concept then an else might make more sense.Agreed on early returns being questionable for "two variations of the same concept" though, these days I generally use if/else for situations like that.
Basically
Allocate resource
try
guard clause (early return)
logic
finally
deallocate
Basically, the try-finally clause guarantees that the finally clause is run when there is an early return.
Influenced by functional programming, I prefer to use the style of: "keep all return statements at the same indentation level" in statement based languages. This way, it is easier to parse as an expression. For example:
if (...) {
let a = ...;
return x(a);
} else {
return y;
}
Can be easily mentally factored into the pseudo-expression: return ... ? x(...) : y;
It is easier to see what the side-effects (or ideally lack of) are. It also makes case analysis easier, and you don't hide the fact that you have 2^{indentation levels} number of possible states.Early return while excusable for some very particular and idiosyncratic error handling examples (e.g. fortified C APIs that accept null pointers as no-ops) in general feels like "cheating", making the code look less complicated than it actually is (it "silently" multiplies the size of your state space without increasing indentation). But most importantly, it puts too much emphasis on control flow: I'd rather emphasize the underlying declarative intent, the state machines, and pre/post-conditions. This is is better achieved, in my opinion, by trying to delay returns, and to try to minimize the amount of code after a branching (this often means having unnecesary else branches, that on the other hand help with readability.) As shown before, this helps mentally factoring the code into reducible expressions and guessing state space and overall complexity.
I agree though that having one single-return is not good, cuz most of the time it forces one to use mutable variables that could otherwise be avoided.
> ...making the code look less complicated than it actually is...
> I'd rather emphasize the underlying declarative intent, the state machines, and pre/post-conditions.
So much this. The goal of refactoring code for readability is not to make it parse more like spoken language (ie, English). Its to aid in understanding and analysis. Having code layed out on the page in a way that mirrors the true control flow graph, makes understanding the CFG easier. Having a data flow graph that mirrors the control flow graph makes understanding the DFG easier.
> ...making the code look less complicated than it actually is...
> I'd rather emphasize the underlying declarative intent, the state machines, and pre/post-conditions.
These statements are strange to me as I personally see early returns as attempts as achieving exactly these goals. I guess people unroll logic in their head in different ways. "flat" to one may appear "nested" to another and vice versa.
foo 0 = 0
foo 1 = 1
foo n =
... if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
goto fail;
goto fail;
... other checks ...
fail:
... buffer frees (cleanups) ...
return err;
[0] https://www.dwheeler.com/essays/apple-goto-fail.htmlNo, it doesn't play well with early return, so avoid mixing them.
def foo arg
return true if arg == 42
puts "got past the guard"
raise "blah"
ensure
puts "ensure always"
end
foo(42)
foo(3)
The "ensure" blocks gets executed whether or not you return early, throw exception or return at the end of the block.All make the "goto manual cleanup at end" less necessary, and make early return easier to use.
Only by doing multiple things in one function you really need to use goto's in C. (again, as far I can judge coding in C, not an expert!)
Yes, in the presence of teardown code at the end of the method, as is common idiom in C, avoid early return. This appears to be the case for apple's "goto fail" code.
However, don't generalise this to all languages.
Personally I think sometimes shorter code, that gets rid of unnecessary bureaucracy, is more readable. For example, C++ recently (in the past decade...) gained range based iteration.
Previously:
for(containe_type::iterator i = my_container.begin(); i != my_container.end(); i++){ do_stuff(i); }
Now:
for(auto& i : my_container) { do_stuff(i); }
This is really context sensitive. Some code benefits from verbosity while other don't.
I don’t agree with him about removing the braces, though. That way madness lies.
I often find inelegant, yet straightforward solutions are generally better options than dense, mathematically pure solutions, simply because inelegant code usually relies on fewer assumptions. Noting that code rarely/never evolves the way we expect it to, apply Occam's razor and strike a balance.
Readability is about clarity of intent and function, as well as sensible structure. Sometimes that means writing slightly more code with more expressive naming so that the code is easier to follow. Sometimes that means using a given language's shortcuts to condense things down.
if (err) {
handleError(err)
return
}
and if (err) return handleError(err)
are equally good. The second one doesn't really make it clear wether handleError returns a value and that value is intended to be returned.Even if it is, I definitely agree that it should only be done if you want to communicate that handleError's return value is meaningful and should bubble up.
if (err) return void handleError(err)
And in non-promise-based async JS, the return value is almost always lost/useless anyway so `return x` has no effect, might as well repurpose `return` for short-circuiting.So people reading this code may have to stop and puzzle out this little pattern.
Here you can replace “ void “ with a new line and have code anyone with basic familiarity with a c-like language can read and understand intuitively.
As a further refinement, I will often take tricky conditional logic and put them into a method that consists of nothing but guard clauses and a return true or false at the bottom of the method. In Ruby you can use ? and ! at the end of method names, so I'll give the method an intention-revealing question-mark name. "Order.used_credit_card?"
I use this in Javascript whenever I can too, though of course it's not as semantically nice as Ruby.
def foo
fail unless bar
return baz if foobar
foobaz
end
[1]: http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Guard...The guys in Gitter chat are quite helpful if you get stuck.
The general information here I think is good and I roughly follow. I have also done almost the opposite and removed as much code outside of the hotpath as possible from the top of the function for performance reasons (guided on profiling). Also there can be times where you jump to a function to see what it does and can't actually see as all of the edge cases come first rather than the bread and butter.
Often you need to be careful when thinking a new paradigm is fixing a problem, like handling the err variable being passed in, and question why there is an err parameter at all. Where does it come from. What are you actually meant to do when it happens? Does every case always just log it, or throw it? Is the logic being performed at the right level? Etc. Sure the answer maybe yes everything is correct but there will be lots of times that the complexity just shouldn't be there.
Horses for courses.
handle(Err, Results) when Err /= undefined ->
handle_error(Err);
handle(_, Results) ->
% etc…
.
though errors would generally be reified as the ad-hoc union of two tuples and would look more like this: handle({error, Info}) ->
handle_error(Info);
handle({ok, Results}) ->
% handle results
.It's not "early return" any more than:
function(err, results) {
if (!err) {
// handle results
} else {
return handleError(err)
}
}e.g. https://doc.rust-lang.org/book/second-edition/ch06-02-match....
So value_in_cents can return a value of 1, 5, 10 or 25.
Saying that it "does not have a return statement" might be truish - but the fact that the "return" keyword is not needed to return values is just syntax.
Saying that there's no early return / multiple exit points is not so much. To my eye there are 4 places where that function can return a value. And the cases might not be simple numbers making the whole thing an expression, later examples on that page show how the cases can differ in side-effects.
https://gist.github.com/jwilk/6e7ecb94b623a82d01cdcd9765f05b...
void function(){
lock_mutex();
void* thingy = malloc();
if(...)
{
return; // Bug!! You forgot to unlock the mutex
// Bug!! You forgot to free(thingy);
}
// Imagine 100 lines of code here
free(thingy); // Hard to remember with 100 lines of code in the way
unlock_mutex();
}
When functions grow to hundreds of lines of code, you need to ensure that all cleanup functions are called. Modern programming languages have features to handle this case "automatically" (Python "with", C++ RAII, Java finally). Which is why its fine today.However, I don't think that coders should shy away from built-up result values for return. It makes the code more friendly to things like copying (I can feel the boos and hisses now), block restructuring, #ifdef'ing (or some equivalent), short-circuiting, and debugging. If you have one easy breakpoint to set that still has a stack so you can peek at a result, it's pretty darned convenient.
So, yes, sure, tend to error out first (this is not news), but don't just litter returns all over the place in code that's going to have to be used and modified by lots of people. The goals for practical code should revolve around the useful lifecycle of that code. Make it readable, portable, mutable, ibleable, ableible, etc.
In many cases, carrying a return value on the stack is a decent way to do this. In some contexts, particularly where branching is expensive, it can have performance implications as well.
I think the statement (ish) “don’t use if/else, use return” deserves some qualification, and one natural place to use it is for errors.
When people start dropping returns all over massive multi-screen branching and looping structures, readability may suffer, and debugability quite often takes a big hit.
In general, I like to use guard clauses for any big at-entry branch asymmetries. If they’re farther down the logic tree and/or unwieldy, it may be time to call some more functions.
With Ruby it's even cleaner because of the postfix conditionals.
def something()
return value2 if error1
return value2 if error2
do_something
end
The return values in case of errors stand out, the conditions for the errors do not clobber the code because they are sidelined.That rule seems to have been so successful, that no modern language I know of allows (the original definition of) multiple entry or multiple return (except for exceptions).
If you read that and thought "does that mean I can change loop iteration order and returns programmatically?!?" and/or "WTF?!", the answer is, yes, to all of it. WTF indeed.
I've never actually seen a non-pathological use for this language feature.
The functional equivalent would be (delimited) continuations and is extremely powerful (but potentially confusing in an untyped context.)
This blog post is about guard clauses and it's good advice. But if you find yourself refactoring huge chunks of code to avoid a keyword, even making code more complex as a result, stop and consider that you may be making things worse, and maybe look at your other habits to see if you're doing the same thing elsewhere.
In getting back into it, I've relied a lot of Laracast's tutorials in both learning Laravel, along with how much PHP has evolved in the past 11 years, but also some of his code quality videos, which included one on this very topic! After seeing that, I immediately started refactoring some recent code I had written with the goal of flattening it to as few indents as possible by eliminating else's and elseif's. Sure enough, I was much happier with the refactored code than I was with the original, even if the output of the code was the same.
Everything is crisp, and distilled down as far as possible. I've learned a lot just from the thoughts it has given names to and elaborated on.
If you're skeptical, there's even free trial episodes that you could give a shot first. (I would recommend "Functional core, imperative shell")
If the function spec is a sequence of complex AND checks which must be satisfied in order to do work, an unnested sequence of “if (!x) return” statements are logically equivalent, have less indentation, and I think it’s much cleaner and easier to read comment blocks above each IF explaining the reason why each check is there.
I also really like seeing the main body of the function which is doing the real work at be bottom without any nesting/conditionals around it at all. It helps you find it if it’s always down there at the end, rather than in the middle indented way to the right.
GOTO has a bad rap IMO. There's a place for it, and the author should probably be using it.
Because go-to is so infrequently a good choice and do frequently used when wrong that most languages don't have it at all, so it's not an option. It may be a reasonable. choice when available, especially if you aren't breaking structure and thereby making it harder to follow the logic.
Obviously it's impossible to completely generalise over, but sometimes it's worth asking yourself if the function is doing too much. In saying that though, early returns are great for input validation.
I try (in C#) to avoid if/else completely (not always possible, but it's a general guide) and I try to work with ternary expressions instead. This forces you to explicitly consider the else case and explicitly state what the intention was. It also makes composition of code blocks simpler and often removes the issues of statement ordering.
You can take it further by using Option\Either\Validation monads to reduce the cyclomatic complexity [1]. This approach can allow for composable and reusable validation computations making the job of validation trivial.
By the way, this is more of a general point about if/else rather than the very simple case of argument validation that the blog talks about. I don't particularly have a major issue with a series of early outs in any function. But if you want to do anything more complex, ignoring else can be a source of bugs.
[1] https://github.com/louthy/language-ext/blob/master/LanguageE...
I agree it's not always easier to debug - but mostly (I find) writing code as pure expressions rather than a sequence of statements reduces the need for debugging massively. So, I don't think it's quite as black and white as you paint.
Debugging isn’t significantly harder, it’s a bit harder. In VS you can break on individual LINQ expression stages, read the unpacked values, read the let values, etc. If I’m composing with Bind functions then I can always break either inside the lambda or the static function being used for the bind operation.
A conversion of the base example would be:
function () {
if () {
x
} else {
if () {
y
} else {
z
}
}
}
Granted it's also worth noting in such a language I rarely find myself using `if` and `else`. More likely I'm doing flow control based on `map` and some version of `getOrElse` (Scala's term for Option that allows you to return an existing value or alternate to a nonexistent value). These abstractions tend to compose better, and lead to more clarity as to what particular data item is leading to the value being one thing or another; e.g.: function() {
requestValue orElse sessionValue getOrElse defaultValue
}
If requestValue is defined, return it; otherwise, if sessionValue is defined, return it; otherwise, return defaultValue. While not all conditions are immediately expressed this way, many of them can be simplified to data flow decisions that can be expressed this way, and the exercise of reworking the solution to use this strategy clarifies the surrounding program as well.(1) often raises an exception or returns an error, failure type, or zero type. The clearest implementation is generally as a guard clause, for the reasons in the neighboring comments and their links.
(2) might most explicitly be represented by changing the boolean to a two-valued enum or (parameter-free) union type or algebraic type; or at least by using a switch statement with cases for true and false. For example, an `ascending` boolean variable could be replaced by a `sortOrder` variable, whose type has values Ascending and Descending. These more explicit alternatives are also more verbose, though. Using a boolean to represent a two-valued type is such a common and readable idiom that introducing an extra type is often a readability and maintenance hit, even if it's more explicit. Similar, using a case statement instead of an if statement might match the underlying math or design, at the expense of verbosity. I generally go with an if statement with two arms, instead of an early return, in to make clear that the branches represent case analysis rather than one normal and one exceptional path.
I am not advocating for the opposite but having return statements in the middle of the code does have its drawbacks. In many instances it makes the code less easy to read. For instance when I write:
If (x) {
Return y;
}
Return z;
I don't see that return y and return z are on the same conceptual level cause their indentation is different. The "else" is implicit, and so as a reader I have to make the effort of thinking "oh that part has already exited, so I'm in the case when 'not x'".Anyways. All in all, just optimize your code for what makes sense and avoid general advice on code formatting.
The danger with early returns is failure to handle all cleanup. This is really a failure of many programming languages. Golang gets this right. But even so, I'd rather have early returns (or early goto-fails) than ever deeper if/else ladders.
Another option that works very well is this:
ret = thing1(...);
if (ret == 0)
ret = thing2(...);
if (ret == 0)
ret = thing3(...);
if (ret)
<handle failure>;
return ret;Working on someone's else code I've found this:
if a, err := func(); err != nil {
return err
} else {
// a exists here
...
}
// a is gone
I would have wrote it differently: a, err := func()
if err != nil {
return err
}
// work with a
I guess it depends on the amount of code to put in the "else" branch, but I thought this was interesting because it looks like someone was avoiding to write the error checking in a different line. ret = thing1(...);
if (ret == 0)
ret = thing2(...);
if (ret == 0)
ret = thing3(...);
if (ret)
<handle failure>;
<common cleanup>;
return ret;
No ever-deepening if/else ladders, and no goto-fails.Alternatively:
if ((ret = thing1(...))
goto out;
if ((ret = thing2(...))
goto out;
if ((ret = thing3(...))
goto out;
out:
if (ret)
<handle failure>;
<common cleanup>;
return ret;
This has a goto-fail, but only one, so it's Go-like.I do a lot of javascript programming, and it's often quite frustrating to not have these and other features available. While I by no means hate Javascript, I have to admit I feel a bit silly about being defensive about it in the past. I just didn't know what I was missing, or didn't see what the big deal was.
I'm glad to hear a perspective like this. I'm fortunate enough to not come across too many people like this, but I'm always utterly baffled by people who get defensive about the smallest complaints about Javascript (of which there are many valid ones that have nothing to do with irrational hatred).
Just a few weeks ago I had some badstalgia when I mentioned I did front-end development to someone. He responded with a rant about how real programmers don't use javascript, and how it was written in ten days, and so on.
But yes, I'm happy to have largely stepped away from the language wars. I occasionally join the #javascript channel on IRC and it's truly astounding how much of the conversation is just people attack or defending javascript passionately, and for very silly reasons.
Yea, it's a crappy feedback loop, where unfairness on either side leads to people being even more strident in their position[1].
> He responded with a rant about how real programmers don't use javascript, and how it was written in ten days, and so on.
Ugh yea, that's exactly the kind of person I'm talking about. I may be dismissive about JS as a language, but the idea that "real" devs don't use it is ridiculous.
[1] though I admit I can't really relate to people taking their choice of programming language personally. To quote pg, "keep your identity small"
Functions are rarely longer than 10 lines, there is no explicit return statement, just a short sequence of data processing and that's it.
It's very much like any logic or debate, where you simply want to prevent getting into any of the details that occur further down the logical chain if you can already dismiss what is to come by detecting the exit condition early. There have been countless occasions where I've heard people having conversations for hours that should failed and exited early due to an exit condition at the very beginning of the conversation.
Why?
1) When using a debugger, it takes more effort to place a breakpoint on the inner statement.
2) If you want to insert statements inside the if block you will be changing version control history for statements other than what you are modifying.
About 2, you may argue that this will be the case anyways because braces are not used... the solution to that is to always use braces.
Then there's another important aspect: handling error conditions is important part of your logic. Do not sweep them under the rug. Proper error handling is a big part of what makes applications robust, they need to be as readable as everything else.
Use a language which has syntax to automatically handle the error handling noise for you (dare I so do notation in Haskell). Go is egregious in that regard, the constant repetition of `if err != nil {…}`.
Did they give any rationale for this?
It was so bad, I remember one time some of the junior programmers literally shaking their heads when they saw some functional programming language examples for the first time because multiple returns are idiomatic there. They couldn't explain why multiple returns were bad, just that they were bad.
I don't find this uncommon though and understanding it's a time saving heuristic. People will accept a "best practice" from an authoritative source without question and only later start thinking about it objectively when it gets in their way somehow.
If your team is ignoring objective arguments though then you're in a cargo cult.
#define CONCAT_LITERAL(x, y) x ## y
#define CONCAT(x, y) CONCAT_LITERAL(x, y)
template <typename F>
struct DeferWrapper {
F f;
DeferWrapper(F f) : f(f) {}
~DeferWrapper() { f(); }
};
template <typename F>
DeferWrapper<F> deferWrapper(F f) {
return DeferWrapper<F>(f);
}
#define defer(code) auto CONCAT(_defer_, __COUNTER__) = deferWrapper([&]() code)
Example of usage: {
Foo *foo;
if (initializeFoo()) {
warn("Foo could not be initialized");
return;
}
defer({
destroyFoo(foo);
});
Bar *bar;
if (initializeBar()) {
warn("Bar could not be initialized");
return;
}
defer({
destroyBar(bar);
});
}
This way, all initialization and destruction happens in one block of the code on the same level of indentation as everything else. Useful for socket code, file handling, encoding/decoding states, etc.I fail to see how "learning proper RAII":
struct FooWrapper {
Foo *foo;
FooWrapper(...) {
foo = initializeFoo(...);
if (!foo)
throw FooException(...);
}
~FooWrapper() {
destroyFoo(foo);
}
}
try {
FooWrapper fooWrapper(...);
}
catch (FooException &e) {
...
}
is easier than Foo *foo;
if (initializeFoo(foo, ...)) {
...
}
defer({
destroyFoo(foo);
})
If this isn't what you meant, could you demonstrate? std::unique_ptr<Foo, std::function<void(Foo*)>> p(new Foo, [](Foo* p) {
if (p) destroyFoo(p);
});
// initialize Foo and set to NULL if failed
It increases complexity a bit because you no longer have a simple pointer, and you can't allocate on the stack anymore (my example should have declared `Foo foo;` with `destroyFoo(&foo)`, sorry for typo.) template<typename TFoo, typename TDestr>
struct FooWrapper {
TFoo foo;
const TDestr destroyFoo;
template<typename TInit>
FooWrapper(Tinit initializeFoo, TDestr destroyFoo) : destroyFoo(destroyFoo)
{
foo = initializeFoo();
if (!foo)
throw FooException(...);
}
~FooWrapper() { destroyFoo(&foo); }
}
Although really that should have been part of the "Foo" class itself, but if you need to deal with external code I suppose that might not be possible.An issue with your wrapper is that it is not generic enough and thus must be written for each Foo-like object. That could likely be fixed with more template arguments, but of course there are multiple ways to initialize C objects like
- `fooInitialize(Foo foo); // expects that foo is pre-allocated`
- `fooInitialize(Foo foo); // allocates and sets the Foo pointer. Annoying, but some libraries do this.`
- `Foo fooInitialize(...); // taking certain arguments`
A single `defer` wrapper allows you implement custom destruction behavior for each instance, which is useful if you want to set flags or log errors in the destruction process.
What worries me is that using 'defer' leaves absolutely no possibility for reusing the code, other than by literally copy pasting it, which just rubs me the wrong way. It goes completely against the DRY principle.
To each their own ¯\_(ツ)_/¯
I put perfecting in quotes, because I believe overengineered, verbose code is something that makes code worse for the reader, writer, and tester. This is why startups are able to accelerate faster than enterprise programmers while they are still able to do this sort of thing.
My point is that RAII is not for everyone, so much so that a popular modern language was developed which avoids it (Go) and includes `defer` instead.
Another thing I avoid doing because it complicates debugging is putting a final calculation on the same line as the return:
return combine(foo.calculate(), bar.calculate());
Some debuggers make it difficult for me to examine the result of the combine() function and/or the results of the .calculate() methods. Better to be explicit: auto foo_result(foo.calculate());
auto bar_result(bar.calculate());
auto combine_result(combine(foo_result, bar_result);
return combine_result;
There, much better, arguably more readable, and I can stop at whatever step I need to.I've had to debug far more code in my career than I've had to write, which has encouraged practices that make debugging as straightforward as possible.
First, do the simple checks "Oh, this function returns an empty set if one of its parameters is null? Do that".
Then go into the main method body. In the main method body, I want to see elses on ifs. I'm okay with putting a return in each branch of the if/else, but what I find bad is when the main body is
```
if(foo)
{
doLotsOfStufff();
return myResult;
}
return bar;
```That pattern I can't stand. That's as confusing as switch-block fallthrough.
For those cases, I prefer to see an else. If you must fall through to a final return while you've had a zillion other returns in your branching logic, then that deserves a comment at least explaining that this is the final fallthrough return.
Like
```
if(foo)
{
doLotsOfStufff();
if(somePositiveCase)
{
return myResult;
}
if (someOtherNeutralCase)
{
doSomePreparatoryThing();
if (heyMoreChecks)
{
return positiveResult();
}
}
}
// this comment here is super important where we
// explain that we couldn't do any positive thing
// so this is a failure state.
return barFail;
```I also disagree with needing a comment, which is why I think not having an else in the first example is great. Having a return statement at the end of a function is obvious, and then all you have to do is scan upwards to find other return tokens. Especially because the final return need not be an error state.
IMHO, the gain is small but not smaller than the cost.
What makes it a "rule" or as sometimes said, a "law"?
It's just a style, and it has pros and cons, cases where it helps, and cases where it hinders.
Because it was a good idea in C.
it's pretty much irrelevant to Java, JavaScript, C#, Ruby, Python, Rust etc.
But the rules stay, for no good reason.
FWIW, I would start making plans to leave a company where any significant amount of my colleagues followed rules like this without challenging them or being able to articulate their rationale.
Is it because we're not really engineers? We're just hackers? I feel like I've seen this discussion every five or ten years or so. Is that just my perception, or do we have waves of interest and knowledge building and then loose it all in some exodus from the industry every ten years that I'm not aware of? One of the threads here ended up exploding into yet another coding style "discussion", the kind I first remember back in 1995, and my attitude hasn't changed since then (just pick one and move forward).
I can only imagine that as soon as B[1] was created, programmers started arguing about whether or not the curly brace should go on a new line.
And that's the real problem here. We are still attempting massive engineering works using the tools of 1969: text files. We have graphical tools with automated assistants for finance, medicine, space flight, car design, but when it comes to the software that we programmers use to create that other software, it's 1969 text files with curly braces. No fucking wonder I keep seeing the same old shit. We should be in outer space right now, but we're still teaching the kids how to hold the sticks to make fucking fire.
But when it's normal programming; YES. Definitely yes.
That said, I entirely disagree with the premise. I find it vastly easier to reason about a function/method that does not return early. To me, this is on par with writing switch statements that don't cover all the possible values: a sin.
When I saw that, I imagined myself spending 10 minutes trying to understand why he does that. But maybe I'm not familiar with a JS best practice here.
Or, as I think of it sometimes, if you have N buffers and M early exits, you need O(NM) free statements. When the code changes over time with more exit paths, you need to change that many places. So best to make M=1 for greatest sanity.
A language that does this for you more automatically (either RAII, or GC) then sure, early exit away. Although, one further point I do like about the more explicit C style is you end up writing code that looks very similar in the failure path and success path, which makes you prepared for when things inevitably fail...
Having a single log line is extremely useful in general as you don't have to filter all the relevant information within other log lines and makes easier to extract a single thread pathway within a multi-threaded application.
however I do like this snippet: > Try keep the “meat” of your method at the lowest indentation level.
but I prefer doing that isolating atomic logics within their own method, instead of having multi-step pathway with early returns
I'd rather change the language.
The advice we give is often prescriptive or absolute, in that you have to avoid doing something, or you should never do something, or there is a best practice. More often than not it strips out the nuance, as if the new position has somehow managed to resolve all of the issues that cause the year--even decade--long debates in the first place, and it is far more often the case that the choice is more than preferential.
And what makes it bizarre is evidenced in the article itself: the original, prescriptive position is neutered in a follow-up edit that claims to take the position in balance because it won't make sense in every situation.
The top comment in this thread talks about the evolution of a programmer; surely one of the greater evolutions of your career in the field is to understand that all of this advice is purely dependent on context and an all-or-nothing approach to rule making isn't going to cut it; in fact it will likely make things worse when aspiring programmers take it as gospel and start trying to shoehorn these best practices into whatever code they can, adding all kinds of linters and such to enforce the rules in all cases and prioritizing style over implementation in code reviews. Code must fit in 80 characters in a line; it must omit semi-colons if it's JS; it must use 2 spaces for indentation; it must be extracted into a method if it has more than 5 lines...no matter what.
As an alternative, I instead propose that you think of good or better practices, also known as reasonable guidelines, and if you're going to build a style guide around them at least try to provide some alternatives should the preferred solution not fit the problem the programmer is currently facing. Treat it as an opportunity for mentoring and not just an instruction manual.
The benefit to automated formatting is the style configuration can be swapped. if I HAVE to see altman brackets my editor can cook that format while my version control can hook whatever format the project requires.
It's important to know both patterns though, so you are able to choose depending on your situation.
“Errors in top if” is good advice but every “else” may need its own (indented) top “if”.
Indentation isn’t that nice but it is a very loud indicator of code becoming too complex, and a lack of indentation is a hint that an error might have gone unchecked.
For the purposes of this post, it is assumed that any errors produced in those calls will automatically bubble up the stack to some caller or to the top level. In my experience, most JS code is not written with fine-grained error checking around every expression, if there's any explicit error handling at all. The example code isn't intentionally omitting any mess, rather it appears to me to be fairly typical.
The "handing" at the top is mainly for checking function preconditions. e.g. "does it even make sense to proceed?". Early returns help to decouple precondition checking from the important logic of the function, which IMO makes these types of checks easier to write and maintain and thus more likely to exist.
Why? Because people with bad judgement were mercilessly brow-beaten into developing better judgement.
We had lots of different styles of code hanging around, and you could tell if code had been written by our CTO, our leads, different guys in different teams, etc. The one rule was "write like a chameleon, and make your code look like the code it is in." (okay... to make that work, the other was "spaces, not tabs")
The code was generally very readable, and it was a very adult codebase. The problem with hard rules is that comprehensive codification of complete rulesets is fundamentally intractable. You're always going to run into areas that don't fit, or the ruleset will be laughably overwrought (and, thus, impossible to put into practice).
You could call it the code-zealot incompleteness theorem...
Of course, if I get to make all the rules and have no obligation to be complete, consistent, or considerate, I can get on board with this plan.
I bounce between many code bases in multiple languages, and this has always been my rule. I first figure out why/how are things done, and then do them the same way within reason. If the existing code has something done in a way that is no longer valid/correct, then we have discussion about changing all of the code.
Your advice is unfortunately common but misguided. Without a firm and intuitive grasp of "rules" and why they exist, one cannot build a framework for making decisions with which one can use their judgment. On top of that, most programmers aren't advanced enough to have more reliable judgment than "the rules" and few working groups can possibly survive a bunch of programmers "using their judgment" all over a nontrivial codebase even when those programmers are that advanced.
A better way of saying it might be: If you don't understand why the rule exists and don't have time to learn, follow the rule. When you follow the rule you may discover why it exists, when you don't follow the rule you will almost certainly discover why it exists.
You end up with code written in several different styles within the same codebase. Assumptions that don't hold, etc. Much harder to reason about and maintain.
Some other comments here discuss resource cleanup issues that can creep into C code with early return.
If you're not coding in C, then more likely it's pure cargo-cult.
In addition, I also like is to avoid if/else when returning bool values:
if (a > b) {
return true;
} else {
return false;
}
Could be simply written as return (a > b); (cond
(early-return-condition early-value)
(second-early-condition early-value2)
...
(t final-else-value))
A cond as the last (or only) form of a body is basically an N-way switch toward multiple exit points, the ones appearing first being earlier.In TXR Lisp I made block have dynamic scope, not only dynamic binding. You can do this:
(defun helper ()
(return-from master 42))
(defun master ()
(helper))
I.e. early return from nested helper functions, without a lot of added ceremony.Speaking of this dialect, here is a function from its C internals, exhibiting a preference for an early return over else:
val flatten(val list)
{
if (list == nil)
return nil;
if (atom(list))
return cons(list, nil);
return mappend(func_n1(flatten), list);
}
If this were in the Lisp dialect instead of C, the same author would write it like this: (defun flatten (list)
(cond
((null list) nil)
((atom list) (list list))
(t [mappend flatten list])))
and certainly not like this: (defun flatten (list)
(when (null list)
(return-from flatten nil))
(when (atom list)
(return-from flatten (list list))
[mappend flatten list])
Even if the implicit block around a function body were the anonymous block, so that (return (list list)) could be used, this would still be eschewed as unidiomatic Lisp. (lambda ()
(cond ((foo) x)
((bar) y)
(t z)))
Such a conditional like COND returns the value(s) of the last form in a clause -> here either the value of x, y or z. for (int 1 = 0; i < length; i++) {
if (checkError(i))
continue;
... // happy path
}
You can also use a return statement inside the for-loop if you want to exit the entire function.Returning early gives me mental overhead of keeping the "elses" on my head, and doing it in Scala makes the code look more verbose.
For complex methods with branching logic you want prominently displayed, you'd end up with:
def myfunction(args) {
// check preconditions, return early
if (x) {
// happy path
} else {
// less happy path
}
}But if the else just contains a bunch of error handling you always have the option of wrapping it in a handler method and putting it near the top in a on-liner.
So break early.
1. It reduce the indention level, makes the code cleaner. 2. Keep the error handling cases at the top of the method, the rest of the method can be written with more ease.
When you break your application fast you generate less bugs.
Is this a common JS idiom?
I don't find that to be the case, though JS isn't my main language.
The sign of someone who doesn't value their time is someone who argues about brace style.
TLDR: use guard clauses (when appropriate for readability and cognitive load)
function(err, results) {
if (err) // …
}
It is assumed that this function will be executed asynchronously, and thus it's not possible for anything to consume the return value anyway.If you want to control the return value, simply return on a new line or use the void operator:
return void handleError(err)