Returning from a long function on the first false condition

Question

I have a long(ish) function of the following pattern:

bool func(some_type_t *p1, another_t *p2)
{
  bool a = false, b = false, c = false, etc = false;

  a = (some_long && expression && \
       (that_deserves | its_own_line));

  b = (another || long_expression && \
       (that_deserves | its_own_line));

  c = and_so && on;

  return a && b && c && etc;
}

As you can see, the return value is true if and only if all flags are true. Hence I can return as soon as one of them turns out to be false. It may or may not be more optimal than the current version, but I want to do it to satisfy my OCPD. I thought of putting everything in a do{}while(0); and breaking out on false, but that looks odd (and I remember seeing something like that in tdwtf).

I do not want multiple return statements or deeply nested if blocks, and I certainly don't want to compromise on readability with a huge if that relies on short circuiting. I can live with goto in this case, but I want to know if there are any other patterns/constructs used in similar scenarios.

Just make it readable. The compiler will do the rest. What is wrong with multiple return statements? I use them all the time. You also should not have very long expressions as a general rule. If you do, then you are doing something wrong - break things out into smaller functions. Don't worry, the compiler will inline things if it can. — Job, Apr 21, 2011 at 16:20
You get credit for remembering the TDWTF entry/entries(?) and not following their example :) What's wrong with multiple return statements? — user7043, Apr 21, 2011 at 16:21
I try to avoid multiple returns as a rule of thumb as they sometimes (not necessarily here) make it hard to debug. If you have just one return statement, you can put a print at the end of the function and see what it returns. — Amarghosh, Apr 21, 2011 at 16:33
If it's C++, you create a log object which logs a message when it goes out of scope, so you don't need to add any explicit print statments. — James McLeod, Apr 21, 2011 at 16:41
Long expressions are much more of a readability problem than multiple return statements. They're also much harder to debug (you can't set a breakpoint in the middle of an expression). I try to avoid using the value of an assignment operation too, again because it decreases clarity… — Donal Fellows, Apr 24, 2011 at 13:45

Brad · Accepted Answer · 2011-04-21 16:27:05Z

10

Maybe I am overusing design patterns or something, but I would write this as

bool func(some_type_t *p1, another_t *p2) {

    return ( descriptive_name_for_a(p1,p2) &&
             descriptive_name_for_b(p1) &&
             etc );
}

bool descriptive_name_for_a(...) {
    return (some_long && expression && \
        (that_deserves | its_own_line));
}

//etc

}

This way people don't have to read the details of the long expressions if they just want to know what func does.

answered Apr 21, 2011 at 16:27

Brad

1,1611 gold badge12 silver badges14 bronze badges

+1 This seems nice in general, but this is a comparison function and doesn't really qualify to be broken down further. I guess I will stick with if(!a)goto EXIT; pattern.
– Amarghosh
May 4, 2011 at 10:32

Add a comment |

Ingo · Accepted Answer · 2011-04-21 17:02:35Z

8

Why not:

return
     (long expression a)
     && (long expression b)
     && (long expression c)
     && (etc.)

This will return as soon as it finds the first false value, thanks to the laziness of &&.

answered Apr 21, 2011 at 17:02

Ingo

3,91118 silver badges23 bronze badges

2

Debugging it may be a problem.
– quant_dev
Apr 24, 2011 at 12:39

Add a comment |

John Kraft · Accepted Answer · 2011-04-21 18:20:58Z

3

I like @Brad's idea about breaking up into functions, but if I couldn't I'd use multiple returns like so. I haven't written C in about 10 years, so this is sorta C# pseudo-code...

Edit: To take into account @Frits' comments; with no attempt to apply DeMorgan's Law...

    bool func(T1 t1, T2)
    {            
        if(!(someLong && expression && (thatDeservers | itsOwnLine)))
            return false;
        if(!(another || longExpression && (thatDeservers | itsOwnLine)))
            return false;
        if(!(andSo && on)
            return false;

        return true;
    }

edited Apr 21, 2011 at 18:20

answered Apr 21, 2011 at 16:37

John Kraft

1,3668 silver badges13 bronze badges

1

Except, of course, that a && clause is redundant if you only evaluate the expression if a is true. Ditto for b &&. And the intermediate returns can probably just return false. Finally, if after this the variables have only one use then you can even just put the whole expression in the if() and get rid of them entirely.
– Frits
Apr 21, 2011 at 16:49
OP specifically says: I do not want multiple return statements
– FrustratedWithFormsDesigner
Apr 21, 2011 at 17:36
@Frits - Good catch. That's what you get when you throw together code with no forethought. :)
– John Kraft
Apr 21, 2011 at 18:15
6

@Frustrated - I'm aware of that. I am firmly against that philosophy. A single exit point almost always clutters the code, IMO. Unfortunately, my hastily thrown together example doesn't illustrate the benefit of multiple returns.
– John Kraft
Apr 21, 2011 at 18:17
@John Kraft: I think we're going to have to agree to disagree on that point (about single vs. multiple returns). ;)
– FrustratedWithFormsDesigner
Apr 21, 2011 at 18:19

| Show 1 more comment

Bill · Accepted Answer · 2011-04-21 16:27:41Z

I am not a C dev, but I typically format this with indentations and line continues to be a single statement with the short circuiting evaluator. Rough guess in C it might look like:

bool func(some_type_t *p1, another_t *p2)
{
  return ((some_long && expression && (that_deserves | its_own_line))) && \
         ((another || long_expression && (that_deserves | its_own_line))) && \
         (and_so && on);
}

To my eye that is a lot more readable anyway.

If the blocks were really intolerably large I would do sub-blocks

bool func(some_type_t *p1, another_t *p2)
{
  return ((some_long && \
              expression && \ 
              (that_deserves | its_own_line))) && \
         ((another || long_expression && \ 
              (that_deserves | its_own_line))) && \
         (and_so && on);
}

But maybe I have spent too much time looking at lambda expressions...

No backslashes needed in C expressions (unless they're in a macro definition). — Donal Fellows, Apr 24, 2011 at 13:45

maaartinus · Accepted Answer · 2011-04-24 11:47:31Z

Use this form using redundant checks as optimization:

bool func(some_type_t *p1, another_t *p2) {
    bool a = false, b = false, c = false;

    a = (some_long && expression && \
        (that_deserves | its_own_line));

    b = a && (another || long_expression && \
        (that_deserves | its_own_line));

    c = b && and_so && on;

    return c;
}

Here, each expression is guarded by a previous one, so there's just a negligible runtime overhead, no matter how long the complicated expression take, since none of them get evaluated needlessly. Most probably, the compiler can eliminate the overhead completely.

That said, I'd prefer multiple return statements, as they're much clearer and less error-prone, IMHO. For example, they allow you to reorder the parts easily.

I would usually go with multiple returns, but if not I think this is a great alternative. — Errorsatz, Feb 12, 2020 at 21:21

Danny Milosavljevic · Accepted Answer · 2011-05-14 12:40:56Z

I certainly don't want to compromise on readability with a huge if that relies on short circuiting.

Huh? I think having one conditional that relies on short circuiting is as clear and readable as possible.

Actually, even without the "if".

Just

return(a < 2 ? b :
       c < 3 ? d :
               e);

as the only line in the function. Almost looks like normal mathematics.

Having multiple statements that look as if they are independent of each other but that really aren't independent at all is unreadable to my eyes (i.e. conditional gotos, temporary bool variables).

But of course it is a matter of personal preference.

+1, but the last sentence is bad - it's a dementi of your former, reasonable statement. — user unknown, May 14, 2011 at 14:51

sanjoyd · Accepted Answer · 2011-04-23 17:16:34Z

-2

How about

#define BREAK_ON_FALSE(x) do { if (!(x)) goto leave_function; } while (0)

void your_function() {
    BREAK_ON_FALSE(first && condition && etc);
    BREAK_ON_FALSE(second && condition && etc);

    /* So on ... */
    return true;
leave_function:
    return false;
}

answered Apr 23, 2011 at 17:16

sanjoyd

1052 bronze badges

2

goto and #define in one function!
– quant_dev
Apr 24, 2011 at 12:40
and return false in a void function.
– user unknown
May 14, 2011 at 15:00
I don't like the #define here, but the goto seems appropriate if any sort of cleanup might be necessary, or if the debugger would otherwise be unable to conveniently trap the early-exit case.
– supercat
May 6, 2014 at 20:15

Add a comment |

Stack Exchange Network

Returning from a long function on the first false condition

7 Answers 7

Your Answer

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

Hot Network Questions

Returning from a long function on the first false condition

7 Answers 7

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

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