14
votes
\$\begingroup\$

Consider the following two code examples:

1.

void foo(boolean cake) {
    if (!cake)
        return;

    // TODO some work.
}


2.

void bar(boolean cake) {
    if (cake)
        //TODO some work.
}

Which one is the most readable/ semantic correct? With the first example you avoid additional indentation on the below code.

Should one return right away when there is no cake or not?

\$\endgroup\$
1

3 Answers 3

19
votes
\$\begingroup\$

Usually you get the advice to avoid multiple exit points in a method. However the reasoning behind this rule is only valid if you have long methods - and then you have more to worry about than exit points.

Make your methods short, then make them even shorter. After this you can use whatever exit style you want, because it will be readable either way.

I came to this conclusion myself, but good books like Clean Code by Robert C. Martin will tell you the same.

\$\endgroup\$
20
votes
\$\begingroup\$

It depends on your intent. Your code should use idioms to match what its actually trying to do.

void foo(boolean cake) {
    if (!cake)
        return;

    // TODO some work.
}

Indicates that !cake is a precondition and as such is insignificant to what the function is doing.

void bar(boolean cake) {
    if (cake)
        //TODO some work.
}

Indicates that cake is an important part of what the function is doing.

\$\endgroup\$
5
votes
\$\begingroup\$

The first approach is what's called a guard. See this article by Jeff Atwood.

Replace conditions with guard clauses. This code..

if (SomeNecessaryCondition) {
    // function body code
}

.. works better as a guard clause:

if (!SomeNecessaryCondition)
{
    throw new RequiredConditionMissingException;
}
// function body code
\$\endgroup\$

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