7 ms·

Another pattern I've found helpful is to have branches "exit early"; if you have code like: if !cache.contains(key) { v = db.query("..") expiry

• slothtrop 8y ago
  Good practice, something I tend to catch only after writing something inefficient.
• tudelo 8y ago
  I believe this is commonly referred to as a guard clause? I tend to prefer writing things this way too... but I could see an argument for putting something in an else block instead of just leaving it there.
• hinkley 8y ago
  Somewhere, in the long dark ago, a bunch of people got upset about long functions with return statements peppered throughout. So we ended up with a Best Practice of one return clause per function. Which is triply stupid because 1) long functions are part of the problem, 2) break to the end with multiple assignments to the same variable is almost as bad. It’s just trading one stupid for another which is a waste of time and energy. And then for many years a loud minority of us, some of whom actually have a vague understanding of how the human brain processes data, insisted on the pattern you describe here. Some of us call it “book ending”. All the returns are fast or proceed to the end. Any method that can’t accomplish either has too high a complexity anyway and gets split until you get either one return, or book ends. [another responder calls them guard clauses, but this doesn’t quite apply in all situations. A clause that handles a default or a bad input is why you can rearrange, but it misse why you should: ergonomics]
  • munificent 8y ago
    The single return best practice comes from the old days of C. Back then, compilers didn't inline function calls as aggressively as they do now, and the overhead of a call was a relatively significant fraction of execution time. That led to longer functions being a good idea. At the same time, C does not have any features for reliably managing resources. No GC. No C++'s RAII. Instead, you have to be careful to release every resource you acquire in the body of a function, along every execution path. In that world, avoiding multiple returns makes a lot of sense because it ensures every execution path reliably reaches the end of the function so it's easier to ensure resources are released. We aren't in that world anymore, so the guideline no longer applies. But it wasn't a stupid rule. It's just unhelpful to take it out of its original context and expect it to apply to a new one. "Check your boots for scorpions before putting them on" isn't a dumb rule in the desert, but it is in Antarctica.
    • hinkley 8y ago
      Ah yes, that's true about C. I think my aggressive distaste comes from seeing people cargo cult this into languages like Java, Python, and Ruby. Makes no sense to turn your code inside out for resource management when you picked a language where you don't have to turn your code inside out for resource management.
    • flukus 8y ago
      Even in C these kind of early returns can be good. In the example above you would be returning before allocating any memory. This makes the memory you do have to manage simpler.
  • wwweston 8y ago
    > Somewhere, in the long dark ago, a bunch of people got upset about long functions with return statements peppered throughout. I got taught this matter of style in the context of 90s C/C++. The general idea was that many return points likely means many point of having clean up any state you've been trying to manage/encapsulate with the function... in particular the state of manually managed memory. Which probably means mistakes. It's a reasonable argument for manually managed memory, loses much of its power once you're not. And these days, most of us are not.
• adamtulinius 8y ago
  Your code should be written this way instead: if !cache.contains(key) { v = db.query("..") expiry = calcexpiry(v) cache.put(k, v, expiry) } return cache.get(k) Only one return, and everything is still in the logical order.
  • jakewins 8y ago
    Yes - the cache example is bad; my point is the general idea of avoiding branches that merge back into the main function. In your example above, the branch does merge back, and clearly that's fine for a small function - and common pattern - like this. But the general idea would be to avoid it if possible, because that removes unnecessary complexity.
  • recursive 8y ago
    I don't think there is anything virtuous about a single return in languages that have catch, finally, and garbage collection.
    • acdha 8y ago
      I think it’s a judgement call: I like clearly indicating when there’s always a single output path but wouldn’t use the single return style in that cache example if it involved an extra lookup operation for a value which is already known.
  • andy_boot 8y ago
    I have used this pattern too. I like it. One small problem is that depending on how your threading and cache works it might be possible for the cache to be cleared after the contains call but before the get.
    • jrumbut 8y ago
      Or for the put operation to finish first every time on development machines and only sometimes in production, thus creating a very tricky bug.
  • 8y ago
• mozumder 8y ago
  These things really should be optimized by the compiler. No reason for it not to.
  • Retra 8y ago
    This is an optimization for readability, not execution speed.
• agateau 8y ago
  I have heard this called early-returns. I like to write code this way too. Proponents of one-return-per-function probably write code in languages like C which does not have GC or RAII so having only one exit point makes it easy to ensure all resources allocated by the function have been released. There are ways to do early returns in C without duplicating resource releasing code though, using gotos (it's one of the few cases where gotos make sense, IMHO). Code looks like this: int aFunctionWhichMightFail() { int result = FAIL; Foo* foo = allocateSomething(); if (!someFunction(foo)) { goto fail; } moreWork(foo); result = SUCCESS; fail: releaseFoo(foo); return result; } The Linux kernel makes heavy use of this.
  • reificator 8y ago
    > There are ways to do early returns in C without duplicating resource releasing code though, using gotos (it's one of the few cases where I think gotos make sense). Gotos like that are fine. Despite the title of the paper I don't think Djikstra's meaning was that gotos should be removed entirely.
    • monocasa 8y ago
      Yeah. Dijkstra was talking about not building your whole program out of if and goto, and using functions and for/while/etc. Particularly he's talking about Dartmouth BASIC that didn't have anything else. Like, imagine the spaghetti lovecraftian horror of a non trivial program that's just if and goto. At the time, there was a particularly vocal camp that essentially made the argument that it shouldn't matter since if/goto is equivalent to what exists in the machine code anyway, and you can technically do all the same things.
      • bitL 8y ago
        Keep in mind adopting any strategy beyond if/goto as is in machine code comes with reduced efficiency. So we trade efficiency for readability. Some talented genius might make a super efficient piece of art by restricting to if/goto only, that no other approach could beat performance-wise. I guess anyone who does low-level machine code execution optimization is in that camp most of the time.
• yoodenvranx 8y ago
  Thank for posting this! I recently had quite a few discussions about this topic with people who insist to _always_ use if/elseif/elsif/... constructs in such situations. I tried to argue that in most situations a "if (a) {return}; if (b) {return}; do_c()" construction is much easier to understand but I had only little success convincing them.
  • Flow 8y ago
    I always say that if you know the answer, why convolute the code? Just return the damn answer and we're done here. I see these early returns as a list of assumptions. It helps making them nicely lined up, with perhaps some comments before each, and an empty line between them. And then when you get below to the, now non-nested, code that actually does some work you have a smaller mental burden needed to understand what is actually done.
• pasta 8y ago
  This. I would amost say: Don't use 'else' (if you can). I also would like to add that it helps to fail early. The article shows a null check for an add function. But the question is: why not throw an exception when the input is not what you expect. This helps to find bugs very early.
  • mdpopescu 8y ago
    If it's a bug, sure. However, it might simply be a "not found" - for example, you're trying to decide whether to insert or update a record. You look for an existing one and don't find it - you shouldn't throw here, because "record not found" is a valid outcome.
  • munk-a 8y ago
    I think 'else' shouldn't be shunned where appropriate, I usually like a lack of else whenever checking function pre-conditions while as I'll include an else (even with both branches returning) if the two branches represent comparable operations. English can be horribly abused to fit anything, but usually saying it out loud can help "If the user id isn't numeric - we're done. If the user doesn't have access - we're done. If the user wants to view the resource in purple shift, let's format it like so and give it back to them, otherwise we'll use a yellow shift and hand it off."
• rasjani 8y ago
  I’ve used Python’s decorators for similar pattern. Syntax and scope is bit different but same structure in general level.
• dwc 8y ago
  I like and use early exits when they fit well, which is often. And in this case I'd probably do as you suggest. But it's worth mentioning that even if you stick with if/else it's often worth it to put the simple case first to reduce mental load: if cache.contains(key) { return cache.get(key) } else { v = db.query("..") expiry = calcexpiry(v) cache.put(k, v, expiry) return v; }
  • muxator 8y ago
    This example makes me whish an early return even more. Wouldn't it be sweet?
• harimau777 8y ago
  Personally, I find the first version easier to read since it makes the structure explicit. I.e. there are two blocks and you will execute one or the other. To me the second version makes it more difficult to differentiate the structure: if (...) { foo() return ... } return bar() from: if (...) { foo() } return bar() I find that when I'm reading code I'm generally more interested in the overall structure than in the specific implementation details. Especially since if the first block was complicated enough to tax my cognitive overhead, then I would probably extract it into a function anyways.
  • joshuamorton 8y ago
    There's a reason the pattern is referred to as "exiting early". Another description might be using guards. Basically, you shouldn't do this if you're doing something complex Basically you can imagine each of the if statements being a "guard" against an condition that might cause you to exit early. This becomes much clearer if you have multiple such clauses, and add the mental note that each conditional should contain only a return statement with a single expression. function foo(bar) { if is_null(bar) { return TypeError } if !validate_input(bar) { return ValueError } if cached(bar) { return cache(bar) } // Added as a workaround for client XYZCorp since... if (bar == "A special value) { return "A special result" } return do_a_complex_thing_to(bar) } You could equivalently structure it as a bunch of else ifs, but there's no real reason to do it that way.
• sbmassey 8y ago
  Personally, it is better to have functions small enough that they fit on a handful of lines, and the if-else structure makes it obvious that you are doing one or another thing, rather than having separate if structures at the top of the function that may-or-may-not exit before you reach the main body of the function.
• sigotirandolas 8y ago
  Unless it’s a one-off thing, it’s also worth considering adding a cache class. This can have the advantages of further decoupling the cache and computation logic, and being able to switch the cache implementation (e.g. to use a no-op cache for debugging). It also hides the “if” by introducing a “cache pattern”: return dbcache.getOrCompute(key, function() { v = db.query("..") expiry = calcexpiry(v) return (v, expiry) });