Geeks With Blogs

News



Add to Google

Tim Hibbard CEO for EnGraph software

 

There is a sales technique where the strategy is to get the customer to say “No deal” as soon as possible.  The idea being that by establishing terms that your customer is not comfortable with with, the sooner you can figure out what they will be willing to agree to.  The same principal can be applied to code design.  Instead of nested if…then statements, a code block should quickly eliminate the cases it is not equipped to handle and just focus on what it is meant to handle.

This is code that will quickly become unmaintainable as requirements change:

private void SaveClient(Client c)
{
    if (c != null)
    {
        if (c.BirthDate != DateTime.MinValue)
        {
            foreach (Sale s in c.Sales)
            {
                if (s.IsProcessed)
                {
                    SaveSaleToDatabase(s);
                }
            }
            SaveClientToDatabase(c);
        }
    }
}

 

If an additional requirement comes along that requires the Client to have Manager approval or for a Sale to be under $20K, this code will get messy and unreadable.

A better way to meet the same requirements would be:

private void SaveClient(Client c)
{
    if (c == null)
    {
        return;
    }
    if (c.BirthDate == DateTime.MinValue)
    {
        return;
    }
 
    foreach (Sale s in c.Sales)
    {
        if (!s.IsProcessed)
        {
            continue;
        }
        SaveSaleToDatabase(s);
    }
    SaveClientToDatabase(c);
}

This technique moves on quickly when it finds something it doesn’t like.  This makes it much easier to add a Manager approval constraint.  We would just insert the new requirement before the action takes place.

Posted on Thursday, June 17, 2010 9:46 AM .NET | Back to top


Comments on this post: Get to No as fast as possible

# re: Get to No as fast as possible
Requesting Gravatar...
I follow that practice myself. I refer to it as "short-circuiting the method", and I write each on a single line for vrebity since they're usually simple conditions, e.g.:

if(c == null) return;
Left by Brian J. Sayatovic on Jun 17, 2010 10:06 AM

# re: Get to No as fast as possible
Requesting Gravatar...
this is a pretty deep post.
Left by jeb on Jun 17, 2010 11:30 AM

# re: Get to No as fast as possible
Requesting Gravatar...
I agree wholeheartedly with this strategy and use it all the time. An added problem is that in some languages/environments/contexts it's not always easy to abort like it by using return within a function.

In those cases you can use a variable to keep track if you need to abort, and then just use a single if around the main part of the code:

var failed=false;
failed=failed || (c==null);
failed=failed || (c.BirthDate == DateTime.MinValue);
if (!failed){
doStuff();
}
Left by Sean Walker on Jun 17, 2010 11:36 AM

# re: Get to No as fast as possible
Requesting Gravatar...
Don't get this the wrong way but sales isn't programming.

The book called "Beautiful Code" talks about the flow a program should respect and this violates those principles.
In the first piece of code, the first and second if statements should be combined because there is no else statement playing and if there were, yes, things could change but you don't have to use "quick exits" to solve it beatifully.

Flow is important; in terms of reading, understanding for humans as well as better chance of making better optimization when compiling.
"Quick exits" look faster for us humans, but I am not sure if that is the case for compilers.

Cheers.
Left by Leo on Jun 17, 2010 11:43 AM

# re: Get to No as fast as possible
Requesting Gravatar...
who cares about compilers dude.
Left by Taylor on Jun 17, 2010 11:58 AM

# As seen on debuggable
Requesting Gravatar...
Referred to as "return home early"

Good comments there as well...

http://debuggable.com/posts/programming-psychology-return-home-early:4811de9f-ae28-49c2-a7dc-2f154834cda3
Left by aeron on Jun 17, 2010 12:06 PM

# re: Get to No as fast as possible
Requesting Gravatar...
First of all because C# short circuits you can combine the first two If statements into

if (c != null && c.BirthDate != DateTime.MinValue)

Second, in the first example as soon as you find out c is null the code execution falls to the end of the method and returns. Some compilers bad at optimization might have 2 jumps instead of one, but I don't think you'll gain any performance increase from your early returns.

Your second example is what has for long been called spaghetti code. Especially the continue statement. You are forcing the person reading your code to think about where the code is jumping to next. I think that is more unreadable than the first example
Left by Bill Kamm on Jun 17, 2010 1:00 PM

# re: Get to No as fast as possible
Requesting Gravatar...
I'm not sure about "flow" being as important as being able to maintain code. return home early /no as fast as possible are much easier to re-factor than elegant code flow. Besides, the compiler will optimize the code for you (some). Sometimes books like Beautiful Code need to be called Beautiful Expense because often times elegance is costly and doesn't buy you anything long term. Beautiful Comments would be a good book.
Left by threeiem on Jun 17, 2010 1:01 PM

# Classic Guard Clause Pattern
Requesting Gravatar...
This is the classic Guard Clause pattern, and it's a great one for simplifying a method and improving readability.

Jeff Atwood has a great writeup on this at coding horror: http://www.codinghorror.com/blog/2006/01/flattening-arrow-code.html
Left by Winfield on Jun 17, 2010 1:33 PM

# re: Get to No as fast as possible
Requesting Gravatar...
Bill - I don't like combining if statements. For me, that is hard to read. I like each line to have it's own "purpose". But that's great if if works for you.

As a point of correction, the continue statement just moves to the next iteration of the for loop without taking any further action. It doesn't fall to the end of the method.
Left by Tim Hibbard on Jun 17, 2010 5:55 PM

# re: Get to No as fast as possible
Requesting Gravatar...
I have always understood this as the 'fail fast' mechanism (or pattern if you prefer).

I have never understood the requirement for a single exit point for every method - the resulting code (where the real work is done nested several levels deep) is not as easy to read as a method that has a bunch of simple 'if' statements that result in a 'return' (or 'throw') statement if they fail.

Randomly returning from a method in the middle of a bunch of logic - ok, not so nice - but determining pre-requisites early in the code and bailing out if they are not met, much easier to read.
Left by ShaneG on Jun 18, 2010 8:41 AM

Your comment:
 (will show your gravatar)


Copyright © Tim Hibbard | Powered by: GeeksWithBlogs.net