Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
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 '11 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? –  delnan Apr 21 '11 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 '11 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 '11 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… –  Donal Fellows Apr 24 '11 at 13:45

7 Answers 7

up vote 10 down vote accepted

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.

share|improve this answer
    
+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 '11 at 10:32

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

share|improve this answer
    
No backslashes needed in C expressions (unless they're in a macro definition). –  Donal Fellows Apr 24 '11 at 13:45

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;
    }
share|improve this answer
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 '11 at 16:49
    
OP specifically says: I do not want multiple return statements –  FrustratedWithFormsDesigner Apr 21 '11 at 17:36
    
@Frits - Good catch. That's what you get when you throw together code with no forethought. :) –  John Kraft Apr 21 '11 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 '11 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 '11 at 18:19

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

share|improve this answer
2  
Debugging it may be a problem. –  quant_dev Apr 24 '11 at 12:39

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;
}
share|improve this answer
2  
goto and #define in one function! –  quant_dev Apr 24 '11 at 12:40
    
and return false in a void function. –  user unknown May 14 '11 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 at 20:15

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.

share|improve this answer

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.

share|improve this answer
    
+1, but the last sentence is bad - it's a dementi of your former, reasonable statement. –  user unknown May 14 '11 at 14:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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