
Reducing Code Nesting  - craigkerstiens
http://eflorenzano.com/blog/2012/01/01/reducing-code-nesting/
======
cheald
Yikes. Lots of return points have always felt like a code smell to me. I've
found that it tends to create surprising and frequently hard-to-maintain code.
I do use early returns somewhere, but I'm rarely happy about it.

For things like the first example, why not something like (in Ruby):

    
    
        # Returns a cached user object either by their user_id or by their username.
        def get_cached_user(user_id = nil, username = nil)
            user =  cache.get_user_by_id(user_id)        ||
                    cache.get_user_by_username(username) ||
                    db.get_user_by_id(user_id)           || 
                    db.get_user_by_username(username)    ||
                    raise ValueError('User not found')            
            cache.set_user(user, user.id, user.username)
            return user
        end
    

You just try all your getters, cheapest first, and when the getter returns a
nil or false value, the next one will be tried. Once one is found, no more are
tried. Then you just set the resulting value in your cache and return the
value. One return statement, no ugly nested ifs, and your unnecessary
statements never evaluate, which is what you want anyhow.

~~~
ericflo
Sigh, I knew using that contrived get_cached_user function with all those
arguments would detract from my point, which is why I tried to couch it in
disclaimers.

My point still stands though: nesting is bad. The takeaway from your comment
is that you can solve the issue sometimes by factoring out smaller more
focused functions instead of returning early, as you've demonstrated.

~~~
mitchty
I have always liked the Linux kernel coding style in this regard:

Now, some people will claim that having 8-character indentations makes the
code move too far to the right, and makes it hard to read on a 80-character
terminal screen. The answer to that is that if you need more than 3 levels of
indentation, you're screwed anyway, and should fix your program.

Yes indentation != nested ifs, but in the end its about the same. I don't mind
a bit of nesting, but I think Linus is onto something with >3 meaning: you've
done something wrong, step away from the keyboard and think about what you've
done.

~~~
ceol
_> if you need more than 3 levels of indentation, you're screwed anyway, and
should fix your program._

A single if statement in a class method would put the indentation at 3,
meaning if you have something like this...

    
    
        class MyClass(object):
            ...
            def my_method(self, param):
                try:
                    self.calculate(param)
                except MyException:
                    if param == 100:
                        ...
                    else:
                        ...
    

you're screwed? I don't see how you can set such a definitive measure of bad
code.

~~~
jrockway
Linux is written in C, so every "top-level" instruction is going to be one-
level deep. In Python, the "top-level" is often the body of a method, which
means it's two deep. So to apply Linus' advice to Python code, you can go four
levels deep:

    
    
        class Foo(object):
            def my_method(self):
                try:
                    if foo is bar:
                       self.quux()
                except:
                     pass
    

This seems acceptable to me. Factor out the try block if you want more
conditionals and use polymorphism instead of conditionals. Rather than
writing:

    
    
        class X(object):
            def quux(self):
                if self.foo is self.bar:
                    self.quux_foo()
                else:
                    self.quux_bar()
    

Write:

    
    
        class X(object):
           def quux(self):
               self.quux_bar()
    
        class FooIsBar(X):
            def quux(self):
                self.quux_foo()
    

No conditionals and your intent is encoded in your class hierarchy instead of
inside some random method.

~~~
sumeeta
FooIsBar doesn't need to be a subclass of X, does it? The polymorphism comes
from the interface (implements `quux`), so inheritance doesn't seem relevant.

Feel free to correct me if I'm wrong. Still trying to learn the difference
between inheritance and "evil" inheritance :)

~~~
jrockway
Python doesn't have traits/roles, so it's hard to give a good example. You
should build classes by composing them from smaller pieces of classes. There
are two good ways to do this; delegation and role-based composition.
Delegation is where you pass the constructor of your class an instance of
another class, and your "foo" method becomes an alias to that instance's foo
method. This allows you to do the same interfaces/roles as the object you're
"wrapping", but still lets you customize the behavior of each method call you
choose to delegate. (The Law of Demeter protects you from other callers
touching the non-delegated methods.)

Roles let you implement pieces of classes without being a full class yourself.
A classic example is a "Comparable" role, that requires the consumer to
implement a "cmp" function, and then provides lt, gt, and eq implemented in
terms of "cmp". You mix this into your class that does cmp, and now it gets
lt, gt, and eq for free. You also get the security of knowing that you're
calling the right method; duck typing will treat a tree's bark and a dog's
bark as the same actions, while roles will let you determine whether you have
an object with a wooden outer layer, or an object that can talk like a dog.

Python's philosophy doesn't seem to push delegation or composition, so we use
inheritance in the above example to stick with Python programmer's
expectations. Just beware of invoking the wrong type of bark; duck typing is
flexible, but it ain't safe.

------
waffle_ss
"... if you need more than 3 levels of indentation, you're screwed anyway, and
should fix your program." - Linus Torvalds, _Linux kernel coding style_
(partly justifying why the Linux kernel style uses 8-space indentation)

<http://www.kernel.org/doc/Documentation/CodingStyle>

~~~
Jach
I was thinking of that quote too. And with Python-style OOP languages, I tend
to increase that to 4 or 5... However, I never really ran into a problem with
it until I started doing stuff in NodeJS and entered callback hell. The
excellent Futures and Async libraries have helped reduce a lot of nested
indentation, much more than getting rid of lambdas does. (And getting rid of
lambdas has its own drawbacks.)

------
aaronblohowiak
> Every added level of nesting is another piece of context that your brain has
> to keep track of.

I disagree entirely. Nesting manifests the conditional-evaluation context in
the presentation of the code. Without nesting, the context shifts must be held
purely in the mind of the programmer (instead of offloading that storage to
the layout of the code.) In dynamic languages especially, I've also found that
early returns lead to more test failing while refactoring (or more bugs if the
code has low test coverage.)

The issue I have with named functions (as shown in the article) is that
context for inner blocks must be passed _through_ the outer blocks, which
means your intermediate function signatures have some irrelevant (to the local
scope) cruft. There are some rough corner-cases where nested code is far less
complicated than juggling context batons.

~~~
statictype
With early returns, there are no context shifts. You're just getting rid of
contexts as and when they are looked at. So there's no question of keeping a
stack of context in mind.

What you say works well when you can physically see the structure in the code.
With deep nesting in large functions, you can't - it's easy to get lost in it.

That said, I can understand what people say about multiple returns leading to
bugs in refactoring - personally, I always prefer early and multiple returns
to nested code, and I've developed the habit of checking the full function for
any exit points whenever I refactor it.

To each his own, I guess.

------
fleitz
You can also place blocks of code in a list and recurse through the list until
the user is found.

    
    
      let cacheUser user =
        cache.set(user.id,user)
        user
    
      let anonymousUser id =
        Some(new User())
     
      let sourceList = [(fun id -> cache.getUser(id),
                        (fun id -> db.getUser(id)),
                        anonymousUser] 
    
      let getCachedUser id =
       sourceList
       |> List.pick (fun source -> source id)
       |> cacheUser

~~~
smosher
I've done this sort of thing to satisfy DRY in general (this kind of nesting
problem is quite repetitive), and to generally make the intention more
obvious. It's not so pretty in certain other languages, but still worth it.

(Edit: I posted before you edited.)

~~~
fleitz
F# always gets me in the editing mode because after I write it the first time
I realize there is some built in function that makes the whole thing so much
easier. I realized my match statement inside of a recursive function was just
reimplementing List.pick

~~~
smosher
(I thought it was OCaml at first... then I realized you had `null` in there
which isn't what you'd want.)

OCaml/F# are too convenient that way. There's a lot of library stuff that you
won't always think to look for.

------
antirez
It's strange that many programmers in this topic feel that multiple return
points are bad. It is very clear how nesting makes code logic more complex,
but why many return points are a problem at all? They don't require you to do
extra work while reading or writing the code, actually it is exactly the
reverse IMHO, every return point remove some possible future state.

Also many times early return is an exact translation of the way we think in
our natural languages: "if that is this way don't continue at all", "if this
precondition is meet return that value", and so forth.

~~~
hxa7241
It is because it is unstructured -- in the Dijkstra 'structured programming'
sense. The flow is no longer nested, so you cannot skim over -- mentally fold
-- parts. It also means the effects of transformations are less 'limited'.

With structured code, if you put a statement after a block it _will_ be
executed (notwithstanding exceptions, but that is a slightly different
matter...), no matter what is changed. But if you have returns, breaks, etc.
that structural condition is lost.

If you have a function with a statement, or block, at the very end, to clear
something up or something, but then someone puts an early return in, the
clear-up will be missed. There is a certain error-proneness there.

The best use of the early-return pattern ('replace nested conditional with
guard clauses' in the Refactoring book) is where that is the only thing going
on in the function. So you are effectively reading/understanding the _whole_
thing as an 'early-return function'.

(And software is very different from natural languages. When you look at
software what you see is not language, but a _machine_. And it has a
particular hierarchical structure.)

~~~
Rusky
Mentally folding sections of code is easier with multiple returns- the code
that satisfies the conditions established by the early return is just the rest
of the function, rather than until the denesting point you have to look for.

RAII, GC, and/or try/finally solve the "error-proneness" of early returns, and
in a language like C you can simulate that with a simple (good use of) goto.

------
gcr
Why couldn't he write the first example like this? Relying on short-circuiting
"or", I find this far more readable and maintainable than his final solution:

    
    
        def get_cached_user(user_id=None, username=None):
            """
            Returns a cached user object either by their user_id or by their username.
            """
            user = (cache.get_user_by_id(user_id)
                    or cache.get_user_by_username(username)
                    or db.get_user_by_id(user_id)
                    or db.get_user_by_username(username))
            if not user:
                raise ValueError('User not found'))
            cache.set_user(user, id=user.id, username=user.username)
            return user

~~~
ericflo
That's another great way to reduce nesting--factor it out into smaller more-
focused functions. I came up with an intentionally contrived example to
specifically induce nesting, but you're right that's a better way to write it.

~~~
vog
Note that the "or" operator has some important downsides. For reducing bugs,
using a COALESCE-like operator is IMHO more feasible. (see also:
<http://news.ycombinator.com/item?id=3415522>)

~~~
masklinn
Considering the semantics of the method (getting a user object or identifier),
I don't think that's an issue. And a falsy OR also allows for using good (and
falsy) null objects instead of nulls in object-oriented languages, an ability
which would be lost (and lead to more reliance on NULLs, a behavior which
should _not_ be promoted) through a stricter selective operator.

The inability to trivially use null objects in conditionals is, in fact, one
of the things I dislike in Ruby (where only `false` and `nil` are falsy).

~~~
yxhuvud
Can you exemplify that inability? What is it you can't trivially do?

------
cellularmitosis
I didn't realize I instinctively picked up the early return habit until I
started doing iOS development, where it seems the convention encouraged by
apple is nesting rather than early returning.

I prefer the early return style. I generally try to put all error handling /
sanity checking code up top with early returns (ie, the preconditions of the
function), so that the "meat" of the code at the bottom of the function can be
more concise and easier to grok. However, this requires a reading style of
"first read the bottom of the function to get the gist of it, and then read
all of the pre conditions above it". Unfortunately I have to explain that to
my coworkers, bu when I do, they seem to understand wha I'm going for.

~~~
jacques_chester
Good old guard clauses. I use them for this exact purpose.

The admonition to use single returns from a function arose from the days when
a) GOTO was the major flow-control primitive and b) Dijkstra and others were
starting to build up a head of steam on the we-should-treat-programs-as-
mathematical-proofs thing.

If you have a single return point out of a function, it's easier to prove
various things about it.

But like a lot of findings, it broke loose of its moorings in the problems of
that time and floated away to lead an exciting care-free life of its own.

Analysis has improved since then, but more to the point, single-return code
often leads to carrying around a bunch of book-keeping variables whose only
purpose it is to artificially prop up that nostrum. They do not relate to the
problem domain: they _add_ to the difficulty of understanding the code.

That's why I feel single-return is a principle that is well past its glory
days. Return from a function where it makes sense.

------
dools
Returning early is, in my experience, a recipe for disaster.

Even in trivial 4 line functions I assign my return values to a variable and
then return that at the end of the function.

This may just be my pedantry left over from when I first studied C at
university, but more than once I've spent a while trying to figure out why a
return value wasn't what I was returning (from some code that had been
previously written by someone else) only to find that there was a return
statement on the 3rd or 4th line of the function.

One could certainly argue that if a function were concise and readable and
commented and all that stuff then this wouldn't be a problem, but that's not
always the case and a little return statement sitting someone in the middle of
the function can really throw you sometimes

~~~
zem
returning early is a good practice for all the guards and sanity checks you
have to perform before the main logic of the function kicks in. if you use it
this way, your function will visually consist of a bunch of small blocks each
with a return, followed by one larger chunk of code that is the "actual"
function, and where you can enforce a single return at the end.

~~~
cema
I concur. This is how I have been structuring function bodies for a while now,
and coworkers tend to easily follow the logic of both the whole function body
and the "actual" work.

------
isntitvacant
I'm increasingly a fan of local named functions in JS, vs. pulling callbacks
entirely out of scope. An example:

    
    
        function compile(filename, ready) {
          return fs.readFile(filename, make_fn)
    
          function make_fn(err, data) {
            if(err) return ready(err)
    
            ready(null, new Function(data))
          }
        }
    

Which neatly addresses the desire to retain closures, while avoiding
unnecessary nesting.

Also, whenever possible, I like to nix callbacks by using Function#bind:

    
    
        res.on('data', accum.push.bind(accum))
    
        // vs:
        res.on('data', function(data) { accum.push(data) })

~~~
ricardobeat
I tend to favor one of these for clarity:

    
    
        Array.prototype.push.bind(accum)
        [].push.bind(accum)
    

(i realize the latter is wasteful, but arrays are so cheap that you wouldn't
notice unless in a hot loop)

------
Zev
I largely try and do the same - use _return_ and _continue_ early and often.
Aside from the things that you mentioned, one more benefit is that doing this
makes it very easy to step through code using a debugger, and see which
condition is failing (rather than having to print out various conditionals, or
add log statements in and recompile/rerun).

------
danvk
Another simple way to reduce nesting is to use "continue" rather than wrapping
entire loops in if statements.

For instance, this:

    
    
      for (file in files) {
        if (isOK(file)) {
          ...  // nested two levels
        }
      }
    

becomes:

    
    
      for (file in files) {
        if (!isOK(file)) continue;
        ...  // nested only one level
      }

~~~
aaronblohowiak
i vacillate between using continue/return to skip code and using indentation.
It is shorter, but I find that it is not as skim-able -- (all?) other control
flow is hinted at by indentation, but continue/break/return is not.

~~~
obtu
return/continue/break really jump out with keyword highlighting.

------
emehrkay
Does anyone else hate multiple returns?

A do/while condition would satisfy this situation. or while True: if False:
break in python

~~~
prophetjohn
It depends on the compactness of the code. If it's a large function, I try to
avoid it, but 100% adherence to "one entry, one exit" kind of misses the
spirit of the idiom. Something like:

    
    
        if (condition)
          return true;
        return false;
    

is no less readable and sometimes more intuitive than the equivalent

    
    
        bool x = false;
        if (condition)
          x = true;
        return x;

~~~
marshray
How about this:

    
    
       return condition;

~~~
ceol
I'm certain he was using True and False as arbitrary values. It's a shame his
point was missed. Please try to focus on his actual argument.

~~~
marshray
I'm not so convinced that it's not a fundamental thing.

Maybe someone wants to post a 'better' example and we'll see where it goes?

~~~
ceol
How about:

    
    
        if (condition())
            return 10;
        return 20;
    

versus

    
    
        val = 20;
        if (condition())
             val = 10;
        return val;

~~~
pka
Well:

    
    
        val = condition() ? 20 : 10;
        return val;

~~~
jonathanwallace
Did you invert the logic of the parent post on purpose?

------
ablerman
Cyclomatic complexity has been used for decades as a measure of software
quality. ( Less is better. ) Any discussion of "reducing code nesting", should
include a mention of it.

<http://en.wikipedia.org/wiki/Cyclomatic_complexity>

~~~
billrobertson42
Cyclomatic complexity (CC) is good as long as it doesn't become the end-all
be-all measure of code. I've run into crazy types that use it as an abusive
bludgeon.

Code that has low CC feels strange if you're not used to it, but there are
some good reasons to like it. One of the things that I find appealing to it is
that by putting things into named functions, you're better expressing your
intent by giving it a name.

~~~
ablerman
I couldn't agree more.

It more important to be aware of it than to live by it. We're all adults here.
( Well, most of us anyway. ) So, as long as we understand the tradeoffs, then
we can decide for ourselves when the complexity is too great.

------
jchonphoenix
I like the article, but changing languages on me midway without warning me
kind of threw me off.

Especially since I didn't notice that I was reading a different language
(subconsciously, code is code) and then it kind of hit me in the face that
python doesn't have curlys

~~~
hkarthik
I felt exactly the same way. I don't know Python, but it's similar enough to
Ruby that I could follow the post.

I was able to grok it, but I think the points could have been made better by
sticking to one language.

------
hayne
There is some empirical evidence that excessive nesting correlates with bugs:
[http://franktheblue.blogspot.com/2011/04/minimize-nesting-
of...](http://franktheblue.blogspot.com/2011/04/minimize-nesting-of-if-
statements.html)

------
tibbe
The "Maybe" monad solves exactly this problem.

~~~
jrockway
Sort of; it's Maybes being composed with <|> rather than with >>=. The code in
Haskell is still extremely ugly, even though there's no indentation moving the
code over to the right:

    
    
       get_user :: Cache -> Database -> Maybe Id -> Maybe Username -> Maybe User
       get_user cache db id name = 
           (id >>= get_user_by_id cache)                     <|>
           (name >>= get_user_by_username cache)             <|>
           ((id >>= get_user_by_id db) <|> 
            (name >>= get_user_by_username db)) >>= 
               \user-> cache_user cache user >> return user) <|>
           Nothing
    

So while I like the look of this code more than I like the look of the Python,
it's still shit. The problem here is not that there is nesting, it's that the
code isn't well-factored.

The first problem is that the function does too much. There should be two
methods, one that accepts a username argument and another that accepts an id
argument. The dispatching between the two types of invocation is one level of
nesting that is completely unnecessary; get_user_by_id(42) and get_user(id=42)
are both equally easy to read, but the latter is much harder to implement
cleanly.

The next problem is that the caching logic is handled in this random data
lookup function, instead of some place where we can reuse the common pattern
of "check cache and return, otherwise fetch, cache, and return". So let's
write that:

    
    
        def cached_lookup(cache, object, method, key):
            # already cached
            if cache.contains(key):
                return cache.get(key)
           
            # not cached; lookup in database and add result to cache
            value = object.method(key)
            if value is not None:
                cache.insert(key, value) # we are punting on calculating the key 
                                         # from the value here, but that is something
                                         # the cache could be taught to do with a 
                                         # decent metaobject protocol.
            return value
    

Now we can rewrite the get_user functions:

    
    
       def get_user_by_id(self, id):
           return cached_lookup(self.cache, self, 'get_user', {'id':id})
    
       def get_user_by_name(self, name):
           return cached_lookup(self.cache, self, 'get_user', {'username':name})
    

Now the functions are separated by concern. The get_user_by_* functions do one
thing: compute a cache key and ask the cache for the object. The cache is then
responsible for the fetching logic. That means if we want to change how
caching works (perhaps to add eviction), we only need to change one thing in
one place. If we made every get_foo_by_bar function handle caching, the code
would quickly become unmaintainable.

So to summarize, nesting draws our attention to problems, but removing the
nesting is not the solution to the problem. Neither are monads. The solution
to the problem is to refactor.

~~~
ericflo
Yeah, I'll agree that the functions weren't very good. I actually had a hard
time coming up with examples that didn't require a bunch of extra explanation.

------
Karellen
One thing I like about early returns/continues, as opposed to nesting, is that
when you're collaborating with people the diffs are much easier to read and
make sense of. You practically never re-indent code.

OK, with changes in nesting you could just use a whitespace-free diff, but
then you can't just apply/commit it. If you apply it, you have to reindent
before committing. (This is tricky in python.) Alternatively, if you get a
full diff, you have to apply it and then extract a whitespace-free diff
yourself in order to read the actual changes properly.

------
epper
First time I write in HN, but I really wanted to give my take on this.

I have to say that I really feel that to get readability, over nesting too,
you often should refactor a bit the code.

I would in fact write the get_cached_user method by using separate methods and
a @cache decorator. Every single function is very readable by itself.

(I'm not fluent in python, pseudo-python follows... but you should get the
idea)

    
    
        def cache(function_to_cache):
            '''
            A Decorator that caches a function result
            '''
            def wrapper(param):
                cache_key = function_to_cache.name + " on " + param
                if cache.contains(cache_key):
                    return cache.get(cache_key)
                else
                    value = function_to_cache(param)
                    cache.set(cache_key, value)
                    return value
            return wrapper
        
        @cache
        def get_cached_user_by_id(id):
            return db.get_user_by_id(id)
        
        @cache
        def get_cached_user_by_username(username):
            return db.get_user_by_username(username)
            
        
        def get_cached_user(user_id = None, username = None):
            user = None
            if user_id:
                user = get_cached_user_by_id(user_id)
            else if username:
                user = get_cached_user_by_username(username)
            
            if not user:
                raise ValueError('User not found')
            
            return user

~~~
kansface
I don't think I'd want to maintain this bit of code although I'd prefer it to
the mess of nesting in the original implementation. Each separate way of
caching gets its own function which is ... ok I guess. Its explicit, which is
pythonic, but overly verbose for multiple methods and we have to draw the line
somewhere.

I think decorators are complex enough that I can't immediately look at the
thing and know what its doing in the same way as early returns. A closure that
takes a function pointer with some introspection magic is just a lot of mental
juggling. At least with your case the complexity doesn't compound and is
pushed as low as possible (as per Code Complete).

A final note: functions can have static members just like classes in python.
fuction_to_cache.name would access the .name member of the function if such a
thing existed (it will be an AttributeError? in this case). You are looking
for function.__name__ I believe.

~~~
epper
I don't think that decorators are too complex to be used usually... In python
it should be a known and understood pattern and therefore it should not scare
at all.

(Moreover it should handle multiple args, and there are better and more
popular implementation such as @memoize)

However my assumption in this specific case is that the project is not small
and therefore that decorator and this "pattern" could be used somewhere else
too. If this is not the case I agree with you that it would just add too much
complexity and it would hence be better to not have the decorator at all. The
get_cached_user_by_username and get_cached_user_by_id would just handle the
cache hit/miss by themselves.

Still it would be much more readable, because the main point I would say is to
separate a long or too nested method in more methods.

    
    
        def get_cached_user_by_id(id):
            user = cache.get_user_by_id(id)
            if not user:
                user = db.get_user_by_id(id)
                cache.set_user(user)
            return user
        
        def get_cached_user_by_username(username):
            user = cache.get_user_by_username(username)
            if not user:
                user = db.get_user_by_username(username)
                cache.set_user(user)
            return user
        
        def get_cached_user(user_id = None, username = None):
            user = None
            if user_id:
                user = get_cached_user_by_id(user_id)
            else if username:
                user = get_cached_user_by_username(username)
            
            if not user:
                raise ValueError('User not found')
            
            return user
    
    

(Thanks for the .__name__ tip... I have not use python for a while)

------
thelastnode
For Javascript, async.js[1] offers some good tools for different kinds of
control flow with callbacks.

[1]: <https://github.com/caolan/async>

------
webreac
Depth of code nesting can be reduced using many syntactic tricks. You have
shown the use of multiple return. This trick is generally considered as a
hidden goto (go to the end of the function). 'GOTO considered harmful' is
practically biblical law amongst many programmers, but it's worth remembering
that he made that statement in the context of an argument with Donald Knuth.
Knuth won:
([http://pplab.snu.ac.kr/courses/adv_pl05/papers/p261-knuth.pd...](http://pplab.snu.ac.kr/courses/adv_pl05/papers/p261-knuth.pdf))
Strict application of usual "good" coding conventions works well in most of
the cases, but always results in poor code for the remaining cases. I think a
relaxed application of coding conventions would be better, but in case of code
review, this requires a lot of efforts to explain why the not compliant code
is cleaner. Writing code compliant to coding convention is frustrating, but is
often easier than to convince stupid bosses. The compliant way to reduce code
nesting is to create small functions (with a single return), even if they are
called only once.

------
simeonf
I'm surprised Eric didn't mention the Zen of Python:

    
    
      >>> import this
      ...snip...
      Flat is better than nested.

------
vog
I think the last Erlang example should be simplified further. It uses two
different values of Resp to signal an error ("Error" and "{timestamp, Start,
Error}"), which can be unified for more clarity:

    
    
        do_some_file_thing(File) ->
            Resp = case file:open(File, [raw, binary, read]) of
                {ok, Fd} ->
                    {timestamp, now(), process_file_data(Fd)};
                Error ->
                    {timestamp, now(), Error}
            end,
            case Resp of
                {timestamp, Start, {ok, Processed}} ->
                    {ok, Start, now(), Processed};
                {timestamp, Start, Error} ->
                    Error
            end.

------
hxa7241
There is something wrong about this.

Code _is_ a tree, code _is_ about nesting. If you do not like nesting, you do
not like code.

Code is not 'text'. You do not read code top to bottom like text. Code has a
structure, and you read that structure.

And if an example of 'improvement' doubles the line count, you have a pretty
good indication you are doing something wrong.

What seems to have happened is a small piece of advice has been taken too far.
The early-return shortcut is reasonable. It is indeed advocated by Fowler and
Beck (who deserve some trust) -- they call it 'replace nested conditional with
guard clauses'. But that is something very particular. It does not suggest
removal of all nesting in general.

~~~
Rusky
Just because code is a tree doesn't mean a deeper tree is better than a
shallower one.

------
rtreffer
Regarding the JavaScript part: I often use small state machines to unroll deep
nesting. You get a better state tracking, less nesting and easier error
handling :-) You should give it a try....

------
jbert
Perl style tends to use early return and also has the benefit of being able to
emphasise the control flow by using a return with a trailing conditional. I.e.

    
    
        return $user if $found;

------
tmeasday
Did anyone else thing that the fact that he needed to do

    
    
            cache.set_user(user, id=user.id, username=user.username)
    

twice in his improved solution a bit of a giant red flag?

~~~
Jare
It was a red flag, but the javascript "curry" solution is like a nuclear
explosion of madness.

A pity, because the general idea and techniques in the article are fine, but
the examples try so hard to be simple that they fail to illustrate the point:
the actual "improved" versions of the code are much worse than the supposedly
broken originals.

------
lukesandberg
This issue comes up a lot between me and my coworker. He is very anti-nesting,
and i prefer methods with single points of return. Obviously it is a balance
to create readable code.

------
jasonlotito
I wrote two articles with the same goal, using simple rules describing when
you should simplify. I also cover cyclomatic complexity, which you seem to
want to describe.

Regardless, the two rules I focus on:

* Idents are intents to modularize <http://www.jasonlotito.com/programming/blocks/>

* Comments indicate future refactoring <http://www.jasonlotito.com/programming/comments/>

They are both covered here:

------
erichocean
Factor (the language) seems to encourage code with very low levels of nesting
-- for most words, none at all.

<http://factorcode.org>

------
andrewflnr
I actually found the second example to be quite nice. Even in my current
sleepiness-fuzzed mental state, I was able to follow it easily. It was almost
refreshing.

I guess that's one of the dangers of using contrived examples. Or maybe it's
just proof that this is as much a matter of taste as anything.

------
cthree
Unfortunately the logic of this article is ass-backward. Multiple return paths
involve jumps and make compiler jump and stack optimizations very difficult.
What is easier to read and follow for a person does not always produce good
instructions for a computer to follow.

------
iamrohitbanga
The following also mentions something on the similar lines
<http://www.kernel.org/doc/Documentation/CodingStyle>

------
jimbobimbo
Code Complete book has a very good section on reducing code complexity and
gives various tips and tricks with the analysis of possible outcomes. Must
read for developers, I think.

------
admax88
Goto

~~~
seliopou
This is, in fact, a great solution in certain contexts:

<http://wwwens.uqac.ca/~flemieux/PRO102/goto_Rubin.pdf>

~~~
cellularmitosis
Agreed, goto can actually be a great way to adhere to DRY when dealing with a
lot of similar error condition checks. Used in the right context, it's bad rap
is undeserved.

~~~
masklinn
> it's bad rap is undeserved.

Its bad rap is entirely deserved. Goto is a fine tool in a few cases, but it's
an awful general-purpose tool when better control structures exist, and its
usage in this case — common when structured programming got its start — makes
code significantly worse.

------
joeyh
This is why I've become very fond of a little advertised part of haskell: the
where clause. Every function can have its own little library of subfunctions,
without polluting the toplevel namespace.

Whenever code starts getting complicated, I break it out into a subfunction in
the where clause, which means I give it a name, which makes my code more self-
documenting, and incidentally keeps the indentation level sane. Breaking
things out into lots of little functions like this often makes it apparent
when the function in the where clause is more generic, and does belong at the
toplevel, and then the code is already separated into a function that's easy
to move out of the where clause (closures do mean additional parameters
sometimes need to be added, but the compiler will make sure you get this
right).

The other nice thing haskell brings to the table, which would be more helpful
in the first example given, is the ability to write your own control flow
functions. For the first example, which keeps trying different actions from a
list until one succeeds, and returns its value, I would write a generic
function to do that. Its type signature would be:

    
    
        firstM :: (Monad m) => [m (Maybe a)] -> m (Maybe a)
    

Of course, I don't need to _write_ that function.. I can just paste the above
type signature into Hayoo, and get directed to an existing implementation:
[http://hackage.haskell.org/packages/archive/darcs/latest/doc...](http://hackage.haskell.org/packages/archive/darcs/latest/doc/html/Darcs-
Utils.html#v:firstJustM)

If I _did_ need to write firstM, I'd feel special to have been the first to
think up such a generic and useful function. So it's win-win-win all the way.
:)

Anyway, the code to use it would look something like this:

    
    
        get_cached_user uid name = fromMaybe nouser <$> find
          where
            cache a = do
              r <- a
              cache_set_user uid name r
              return r
            nouser = error "user not found"
            find = firstM
              [ cache_get_user_by_id uid
              , cache_get_user_by_username name
              , cache $ db_get_user_by_id uid
              , cache $ db_get_user_by_username name
              ]
    

As another example of this refactoring of control flow, looking at the cache
function above I realized I've written those three lines several times before.
So I just added this to my personal library:

    
    
        observe :: (Monad m) => (a -> m b) -> m a -> m a
        observe observer a = do
          r <- a
          observer r
          return r
    

(I seem to be the first person to think of this function.. yay!)

With this, the "cache" function can be written as just

    
    
            cache = observe $ cache_set_user uid name

