0

By Guard Statements I mean something similar to the first part of the function:

def doSomething(String something)
{
    // Guard Statement
    if(!something)
    {
        return false
    }

    // more stuff
}

Say you might have several parameters, maybe you need to log that the method was called with a null parameter, or maybe throw a specific exception.

Clarification: I've read that both Guard Statements and smaller functions are good design choices, but it seems like if you include Guard Statements your functions won't be small.

By smaller functions I'm referencing Robert C. Martin's smaller function suggestions in Clean Code.

I guess what I'm asking is it worth it to include Guard Statements in a function even though it makes the function longer. I understand the answer could very based on if it's a public API or private function, for this purpose it's not a private function but a service in a MVC application, so I guess that makes it public?

  • 8
    And what is the question? – cmaster Jun 8 '15 at 20:01
  • 1
    Trivially, including guard statements increases the size of a method. Whether that makes something "not small" depends on your definition of "small" and how many guard functions you need. If you find yourself needing dozens of guard statements, I would tend to expect that something is wrong with your design. If you have repetitive code in your guard statements, you can obviously factor that out as well. – Justin Cave Jun 8 '15 at 20:05
  • Recently I start using this coding style: converting those guard statements into "single line conditionals" as mentioned in Telastyn's answer. Example: ThrowIfBitmapNotGray8(bitmap), which subsequently calls ThrowIfBitmapNull(bitmap). In some cases, one can reduce the number of guard clauses down to one per argument, which I think is the minimum possible. Remember that there are other meaningful validations other than null checks. – rwong Jun 9 '15 at 12:05
2

Don't think of short functions in terms of absolute LOCs number. It is irrelevant, because:

void demo(int a)
{
    if (a < 0)
    {
        this.dealWithNegative();
    }

    for (int i = 0; i < a; i++)
    {
        yield this.doSomething(i);
    }
}

and:

void demo(int a) {
    if (a < 0) this.dealWithNegative();
    for (int i = 0; i < a; i++) yield this.doSomething(i);
}

are the same in terms of complexity: the second variant is not three times better because of its LOC of 4 lines versus 12 lines in the first example. (Actually, the second example is much more error prone, and so worse, but this is a different subject.)

Think of short functions in terms of how long would it take you to understand them. When you start working with a method, you tell:

“Oh heck, it would take me two hours to figure out what is all this code about.”

and not:

“Well, there are 124 lines of code, so it should take me from 96 to 112 minutes to figure this code out.”

right?

While bigger LOC usually leads to methods which take more time to understand, there is no strict correlation between two factors. For example, how long would it take you to understand a method which maps a value to another, containing 50 values (and so 53 LOCs)? Would that change if the map contains only 10 values? What about 300 values?

Guard clauses don't necessarily make methods longer, because they are simple to understand and don't take too much of your time when you prepare to work with the code. They may not even increase the absolute number of LOCs, because most stuff they deal with is otherwise scattered in your code. For example, what is simpler:

  • The code with guard clauses:

    void demo(int a, int b)
    {
        if (a <= 0) throw new ArgumentOutOfRangeException(...);
        if (b < a) throw new ArgumentException(...);
    
        var c = b / a;
        this.doSomething(c);
    }
    
  • or the contrived logic of the similar code without them:

    void demo(int a, int b)
    {
        if (a != 0)
        {
            // The division is safe: we won't have division by zero here.
            var c = (double)b / a;
            if (c < 1.0)
            {
                // We shouldn't have `c` inferior to 1.
                throw new NotImplementedException();
            }
    
            this.doSomething((int)c);
        }
        else
        {
            throw new ArgumentOutOfRangeException();
        }
    }
    
3

A few options:

  1. Don't use guards. Seriously, if a null parameter is going to blow up, just let it blow up. Also, while it's possible for some naughty programmer to call internal functions/services directly with bad data, you maybe don't need to guard everything.
  2. Single line conditionals. This is one place where a single like if(string.IsNullOrWhitespace(something)){ return; } is good. Keeps cruft out of the way.
  3. Sane defaults. something = something ?? string.Empty; (or similar) is much shorter, and in addition, it makes your function less fragile to bad input. Some people hate this though.
  4. Helpers. In C#, you can use expression trees to do something like Guard.Range(()=>x,1,10); which will throw while being more concise and more DRY.
  5. Don't care. "Small functions" can mean "less going on" in addition to "taking less space on the screen". The former is more important than the latter. While big space eating guards are annoying, they're easy to read. If you can get over that annoyance, then you can focus on what really matters - keeping your code easy to maintain by keeping the scope (not size) of functions small.
  • 2
    6. Private functions created internally during refactoring may not need guards if the class is properly designed, since they don't have an external API. – Robert Harvey Jun 8 '15 at 20:13
  • @RobertHarvey - that'd fall under #1 for me, though I've gotten the argument in the past "sure, it doesn't have an external API now...". I think it's garbage, but I'm not sure how to refute it. I go YAGNI, they go "but input checking is good", I then exit the discussion and proceed to not check inputs on solidly internal functions. – Telastyn Jun 8 '15 at 20:15
  • If we're not going to bother unit testing our internal functions (i.e. "test the public API only"), I don't see why we should bother with guard functions either, unless and until such time that the function is made public for external consumption. – Robert Harvey Jun 8 '15 at 20:18
  • 1
    -1 if only for the statement of "Don't use guards." I will grant that some application domains don't necessarily need guard checks. But many functions should be written with a defensive mindset, and guard checks are an important element of that mindset. For the same reasons that we don't pass un-inspected elements into our SQL queries (thank you! Little Bobby Tables), guard checks provide an important step to ascertain that the passed data is clean and reasonably trustworthy. – user53019 Jun 8 '15 at 22:12
  • @GlenH7 - I would argue that the goal isn't to inspect elements, but to make any input to a function be handled gracefully. Explicit, manual guard checks are one of the worst ways of doing that though.(#3 for example provides a common alternative) – Telastyn Jun 8 '15 at 22:29
0

"Functions should be small" is not an absolute requirement. The time to read and understand a function could be modeled as a constant, plus some small constant times the number of lines of code squared. Many small functions are hard to understand. Few enormous functions are hard to understand. There is middle ground that you need to look for.

But then, you are not looking for the least complex code. You are looking for the least complex code that solves your problem reliably and in an acceptable time. If guard statements are needed to make your code reliable, then it doesn't matter how much they add to the size of a function.

It has been said "don't use guards". Determine what happens without guards and how bad that is. If you figure out "if I don't use a guard, then the code will crash in development" - don't use the guard. Wait for the crash in development, fix the reason for the crash. Define the interface for your function, document it, and if the caller doesn't respect it, feel free to crash (sometimes you may use a guard to invoke the crash). If you figure out "if that guard ever triggers, then I'm in a totally unexpected situation". Feel free to crash if that is better than running code without any idea what the result will be.

Where you should have guards is when you expect that things could be wrong. Typical: Processing JSON data from an unknown source. Anything could go wrong, and you want to use guards to tell the caller "your data was rubbish" instead of "I processed all your nice data".

0

Encode your preconditions in the type system so that a program which violates your guard can't compile. For example (scala)

object PositiveInt {
  /** Returns a positiveInt wrapper or None */
  def apply(i: Int): Option[PositiveInt] = if (i >= 0) Some(PositiveInt(i)) else None
}
case class PositiveInt private (value: Int) extends AnyVal // Value class to avoid extra object creation

def squareRoot(i: PositiveInt): Double = math.sqrt(i.value)

In this case I make it so that it's not possible to invoke squareRoot on an int which we haven't verified as being positive. The constructor is private so the only way to get one is to use method returning Option and handle the case where it returns None - failure to do this means the program doesn't compile.

  • This is a great approach in functional languages, but in imperative languages like C# and Java it requires a LOT of boilerplate and it's not a common idiom, so people reading your code will be confused. Could be a nice convention in a team, but I'd think twice before doing this. – sara Mar 29 '16 at 7:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.