4

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.

9
  • 7
    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
  • 6
    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
  • 1
    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. Apr 21, 2011 at 16:41
  • 2
    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… Apr 24, 2011 at 13:45

7 Answers 7

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.

1
  • +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
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 &&.

1
  • 2
    Debugging it may be a problem.
    – quant_dev
    Apr 24, 2011 at 12:39
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;
    }
6
  • 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 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). ;) Apr 21, 2011 at 18:19
1

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...

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

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.

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

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
  • +1, but the last sentence is bad - it's a dementi of your former, reasonable statement. May 14, 2011 at 14:51
-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;
}
3
  • 2
    goto and #define in one function!
    – quant_dev
    Apr 24, 2011 at 12:40
  • and return false in a void function. 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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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