Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

[C++] Never have more than 1 return statement?

  1. Mar 22, 2014 #1
    I know someone at Google who says you should never have more than 1 return statement in a function. That seems ridiculous to me.

    Let's take a simple example. Suppose we need a function that finds the maximum value of any C++ array.

    I can do

    Code (Text):

    template <class T>
    bool max (T* arrPtr, size_t n, T highest)
    {
        if (n == 0)
        {
            return false;
        }
        else
        {
            for (T* i(arrPtr), j(arrPtr + n); i != j; ++i)
                if (*i > highest)
                    highest = *i;
            return true;
        }
    }
     
    or, equivalently,

    Code (Text):

    template <class T>
    bool max (T* arrPtr, size_t n, T highest)
    {
        bool retval;
        if (n == 0)
        {
            retval = true;
        }
        else
        {
            for (T* i(arrPtr), j(arrPtr + n); i != j; ++i)
                if (*i > highest)
                    highest = *i;
            retval = false;
        }
       return retval;
    }
     
    Apparently, the first is considered bad because it has two return statements? The second seems very n00b-esque to me. (I know there's an even better way to do the whole function.)
     
  2. jcsd
  3. Mar 22, 2014 #2

    ShayanJ

    User Avatar
    Gold Member

    No, the first one is completely OK. I have no problem with it.
     
  4. Mar 22, 2014 #3

    DavidSnider

    User Avatar
    Gold Member

    This advice was popularized in "Code Complete" which suggested you minimize returning early except when it would enhance readability.

    I find the biggest benefit of the "one exit point" that you only need to set one breakpoint when debugging.

    Though if you are keeping your functions small and uncomplicated this probably won't be much of an issue anyway.
     
  5. Mar 23, 2014 #4

    AlephZero

    User Avatar
    Science Advisor
    Homework Helper

    Personally, I would be more worried about the value not returned in "highest" than about the number of return points :smile:
     
  6. Mar 23, 2014 #5

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    The "single point of entry / single point of exit" rule goes back much further than Code Complete. This rule is very old. The first part of the rule gives a clue as to how old this rule is. The facility to define alternate entry points to a function doesn't exist in C or C++, or in most other languages for that matter.

    It's an old rule based on archaic concepts. It deserves to die.


    This benefit goes away with debuggers such as gdb that let you set a break point on the function's closing bracket.

    Another supposed advantage of a single exit point is cleanup. Suppose a function creates and connects to a socket, opens a C-style stream, allocates an array via new, copies data from the socket to the stream, and then cleans things up. There are lots of things that can go wrong here. There's no point in continuing if the socket won't open properly, and then there's no stream to close, no array to delete. There are multiple ways in which the socket connection can fail, and the socket can fail while reading from it. The cleanup can be *ugly*. In fact, some advocate using goto to branch into the right part of the cleanup section.

    There is a way around this cleanup problem in C++: Use RAII. Do this consistently and once again the impetus driving a single exit point vanishes.


    I'm a fan of "early exit". Suppose you've done a good job defining your functions in terms of preconditions, invariants, and postconditions. What to do about those preconditions? Suppose you are reading a complex function and the first thing you see are a handful of statements of the form
    Code (Text):

    // Handle preconditions.

    if (! precondition_1) {
       issue_error_message("Precondition 1 failed");
       return;
    }

    if (! precondition_2) {
       issue_error_message("Precondition 2 failed");
       return;
    }
    You know exactly what these checks are doing: They're ensuring the preconditions are met. That's the principle of least astonishment at work. The alternative of a single exit point can be more astonishing. You have to scroll all the way down to see that these preconditions just exit. Even worse, you may have to diagram some rather convoluted logic introduced just to comply with the "single point of entry / single point of exit" rule.



    Yep. The arguments I've seen for single point of entry / single point of exit are usually straw men. For example,
    Code (Text):

    // 100s of lines of code elided
    if (some_test) {
       return ERROR_CODE_1;
    }
    // 100s of lines of code elided
    for (i = 0; i < first_limit; i++) {
       // 100s of lines of code elided
       if (some_other_test) {
          return ERROR_CODE_2;
       }
       // 100s of lines of code elided
       for (j = 0; j < second_limit; j++) {
          // 100s of lines of code elided
          if (yet_another_test) {
             return ERROR_CODE_3;
          }
          // 100s of lines of code elided
       }
       // 100s of lines of code elided
    }
    // 100s of lines of code elided
    return SUCCESS;
    That this code has four exit points interspersed randomly throughout several hundred lines of code is only the tip of the iceberg of the problems associated with this block of code.
     
  7. Mar 23, 2014 #6
    The rationale behind it, is that if someone who is unfamiliar with the code wants to do something at the end of the function for all cases, then they can miss a return point and introduce a bug.

    People who analyse this sort of stuff can estimate how often mistakes like this happen and their cost to a project. Future languages may even make it impossible.

    I do it some cases, but then I'm mindful of how someone might modify my code and what I want the compiler to do with it.
     
    Last edited: Mar 23, 2014
  8. Mar 23, 2014 #7

    AlephZero

    User Avatar
    Science Advisor
    Homework Helper

    How did they know they wanted to do something at the end for all cases, unless they traced all the possible ways they could get to the one return point?

    Thinking or assuming they wanted to do something in all cases (e.g. by believing or misunderstanding the documentation, but not actually reading the code) might also be a bug!
     
  9. Mar 24, 2014 #8

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    I've heard lots of excuses for the "single point of entry / single point of return", but I haven't heard this one before. That's pretty lame.

    Name one. Citation needed.

    This thread isn't about speculative directions in computer language development. It's about C++.

    That said, those future languages will most likely formalize the concepts of preconditions and postconditions. That isn't the case in C++, which is why you will oftentimes find code that implements those preconditions in the form of early exit or throwing an exception.

    In addition to those early exits, another common use for multiple returns in C++ is a search function. These two paradigms, using return statements for preconditions and a successful search, are widely used and widely accepted amongst professional C++ programmer.

    Can return statements be abused, make code more confusing? Of course. Just because they can do so when used inappropriately does not mean they almost always do. More importantly, there are places where a return statement can enhance readability / understandability.
     
  10. Mar 24, 2014 #9
    When you're working with other people's code, you don't always fully understand the functions that you're modifying nor do you always need to. It's not ideal, but many experienced software developers have experienced, it at some point.

    It's certainly not lame if you're working on very large scale, safety critical projects.

    Jean Ichbiah et al took this very seriously, for example.

    Agreed. Very small functions and functions that are clearly structured for early-outs, are good examples. If you care about performance, they can also be used to coax the compiler into generating more optimal code, in certain cases.

    Regarding the OP, both of those functions are completely horrible. More structure to your programming will certainly help and in this case, a single exit should actually make your code simpler. Perhaps then you'll actually see the bugs in it. I can see 2 reasons why the first function doesn't even work straight away, one very dubious omission and one extra bug introduced by refactoring it for the second function.
     
    Last edited: Mar 24, 2014
  11. Mar 24, 2014 #10

    jim mcnamara

    User Avatar

    Staff: Mentor

    McCabe's static code metric algorithm actually counts function returns as part of determining 'cyclomatic complexity' - a measure of the feasibility of code testing, primarily a result of code branching. It is old.

    McCabe T. J., "A Complexity Measure". IEEE Transactions on Software Engineering 1976

    My point is not about McCabe's approach, good or bad, but the fact that the algorithm counts "extra" return statements as negative dings the to the overall result. More returns are supposed to be bad. I believe this kind of thing has fostered the idea: 'more than one return in a function is bad'

    Also see: http://en.wikipedia.org/wiki/Cyclomatic_complexity
     
  12. Mar 24, 2014 #11

    jim mcnamara

    User Avatar

    Staff: Mentor

    @craigi - on PF, and in Science in general, a citation means just that: author,( journal or book) title, article title or chapter citation, date.

    The most common reference for Jean Ichbiah is understandably: Ada
     
  13. Mar 24, 2014 #12
    Sure, but I'm already happy with my contribution to the thread. Thank you for your contribution too. Searching for citations to support what we both already know seems fruitless and I'm content for anyone who doubts it to disregard it, out of hand.
     
  14. Mar 24, 2014 #13

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    That is what exactly what I do: Projects in the MSLOCs that can result in billions of dollars of damages, loss of life, and loss of national prestige.

    But don't take my word for it. Let's look at a coding standard for a large scale, safety critical project that has been widely promulgated throughout the large scale, safety critical C++ community, the Joint Strike Fighter C++ Coding Standard (http://www.stroustrup.com/JSF-AV-rules.pdf) (emphasis theirs):
    4.13.2 Return Types and Values
    AV Rule 113 (MISRA Rule 82, Revised)
    Functions will have a single exit point.

    Rationale: Numerous exit points tend to produce functions that are both difficult to understand and analyze.
    Exception: A single exit is not required if such a structure would obscure or otherwise significantly complicate (such as the introduction of additional variables) a function’s control logic. Note that the usual resource clean-up must be managed at all exit points.​
    Note that this is a will rather than a shall requirement, and also note that the rule has an explicit exception.


    That is not a citation.


    Whether return statements raise the cylcomatic complexity is subject to debate. A return statement is equivalent to goto __close_brace. It's just another edge in the local graph. On the other hand throwing an exception, calling exit, or calling some project-specific function that acts like an exception (i.e., the function doesn't return to the caller) is an alternate exit point, and they do bump the complexity.
     
    Last edited: Mar 24, 2014
  15. Mar 24, 2014 #14

    jedishrfu

    Staff: Mentor

    For references, the Apple/IBM Taligent project published a Taligent's Guide to Designing Programs, Well Mannered Object Oriented Design in C++

    While Taligent is defunct, the guide is still useful for coding standards.

    On page 52 Things to Avoid: Don't Use goto

    They mention avoiding the use of returning from the middle of a procedure as something that should be reviewed with the project architect. The reasoning is that it could subvert the meaning and correctness of the code requiring you to read all the relevant code to see whats going on.

    https://www.amazon.com/Taligents-Gu...2607&sr=8-1&keywords=taligent+guide+to+design
     
    Last edited by a moderator: May 6, 2017
  16. Mar 24, 2014 #15

    AlephZero

    User Avatar
    Science Advisor
    Homework Helper

    The problem with over-simple "rules" is that they are too easy to subvert, as in the OP's code where "goto end-of-function" is replaced by the state variable "retval" which has no other purpose in the code. One of my work colleagues used to call this style of coding "the if-come-from statement" or "backwards programming" (and less complementary names which probably don't meet the PF posting guidelines!)

    Of course there are much more creative ways to subvert a "no gotos" or "only one return" rule than he OP's method, as D.H. already mentioned - throwing user-defined exceptions, etc.
     
  17. Mar 24, 2014 #16

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    I've only seen the use of goto "justified" in a tiny number of cases.

    Case 1 (very rare): The code gets in trouble deep inside multiply nested loops. This can happen, for example, with some bizarre corner cases in singular value decomposition. While SVD is typically very robust, there are some weird corner cases that defeat this robustness. The problem is that the problem isn't uncovered until deep in the bowels of the SVD algorithm. There would be no problem with this problem if C/C++ had a multilevel break capability. The goto statement provides a way to emulate this missing capability.

    Case 2 (much more common): Some fool of a manager has mandated the single point of entry / single point of exit rule, with no exceptions allowed. Some programmer complains that the only alternatives are to add unneeded complexity to the code or to use gotos. The manager's response: "That's right. This is one of those cases where gotos are the preferred implementation." :bugeye:


    I for one much prefer to see preconditions dealt with up-front in a manner that clearly calls them out as such. A small (one to three) statement blocks of the form if (simple_test) { log_error(); return; } at the head of a function won't bug me in the least -- so long as those preconditions are clearly documented. Those up-front statements tell a nice Principle of Least Astonishment story.

    On the other hand, I've been asked to participate in a code review of a function with over 200 lines of code and a cyclomatic complexity in excess of 20. Those numbers alone bedeviled me beyond belief. That that tangled mess of code of contained buried return or goto statements will just cemented the deal.
     
  18. Mar 24, 2014 #17
    I may be the only one here who really remembers where the "one exit" rule came from.

    It was in rebellion to "spaghetti code" - especially prevalent among FORTRAN programmers doing maintenance coding. It came with strict adherence to the basic programming structures ("structure programming").

    There is clearly some absurdity to it - as there was with the strict dictates of structure programming. Although it was (and is) true that you could not have spaghetti code with strictly structured programming, what you do get is truly obscure flags to signal which path you want to take.

    Personally, I do subscribe to "GOTOless" code - although I wouldn't go so far as to forbid it. My main concern is being able to follow the code - and until you've tackled a system with hundreds of pages, you;re probably not going to catch on to what that means.

    Basically, you should realize that there are many ways to implement an algorithm and the one you want to pick is the one that has an easy to follow human-language "story".

    As far as single exit coding, like craigi, I commonly deal with safety related or military mission-critical systems and so I give the auditors their due. Some people feel very strongly that there should be only one exit - and I do not care to spend my time fighting them.

    So, here is how I would implement the code - allowing for one exit while retaining "sensibility":
    Code (Text):

    template <class T>
    bool max (T* arrPtr, size_t n, T highest)
    {
        bool bOkay;

        //
        // Check valid input (or any other description of why your are checking "n")
        bOkay = n != 0;
        if(bOkay)
        {
            for (T* i(arrPtr), j(arrPtr + n); i != j; ++i)
                if (*i > highest)
                    highest = *i;
        }
        return bOkay;
    }
     
    Sometime the solution is not quite so simple. Very commonly, there will be many, many exit conditions. The most common solution for avoiding numerous "returns" is to start nesting if statements - one or two levels of nesting for each error condition checked. For example, you're opening a file and you want to check the file name, whether it can be opened, whether a malloc works, whether the filesize is adequate, whether the first 10 bytes are "MyFileType", etc. Wouldn't you love to go "if(!malloc(...)) return MYERR_MALLOC;"?

    Here's one not-that-bad method - using an enumeration variable "eErrorState":
    Code (Text):

    enum { MYERR_NOERROR=0, MYERR_BADNAME, MYERR_NOFILE, MYERR_MALLOC } MYERR;
    MYERR eErrorState;

    eErrorState = MYERR_NOERROR;
    //
    // Check filename
    if(!eErrorState) {
      ...
      if( error condition ) {
        eErrorState = MYERR_BADNAME;
      }
    }
    //
    // Check file
    if(!eErrorState) {
      ...
      if( error condition ) {
        eErrorState = MYERR_NOFILE;
      }
    }
    ...
     
    Not bad, but I prefer this:
    Code (Text):

    enum { MYERR_NOERROR=0, MYERR_BADNAME, MYERR_NOFILE, MYERR_MALLOC } MYERR;
    MYERR eErrorState;

    eErrorState = MYERR_NOERROR;
    //
    // Non-loop to make for easy "break".
    for(;;) {
      //
      // Check filename
      ...
      if( error condition ) {
        eErrorState = MYERR_BADNAME;
        break;
      }
      //
      // Check file
      ...
      if( error condition ) {
        eErrorState = MYERR_NOFILE;
        break;
      }
      ...
    }
    return eErrorState;
     
     
  19. Mar 24, 2014 #18
    This is a beautiful illustration of 2 things that I was talking about earlier in the thread.

    Firstly, it's the simplification that I was referring to that the single exit point paradigm offers in this case.

    Secondly, it's easy to see from it how programmers make modifications to functions that they don't fully understand. This shouldn't be taken as a critisism of .Scott, rather just as an observation of how a programmer typically modifies unfamilar functions. In this case he has rightly presumed that he could do no harm to this function by modifying it to have a single exit point, but has also inadvertently copied the 3 serious issues, from the OP's original function, that I was referring to earlier.

    Now this is an incredibly simple function. In the case of a more complex function, it really isn't a huge leap of the imagination to see how a programmer attempting make a modification to a function without understanding it in its entirety actually introduces new errors in the false belief that they have done no harm. A more structured programming style defends against these scenarios.

    I actually take a very liberal approach to coding standards, but they can serve as a very useful learning resource for inexperienced programmers. Enforcing coding standards is no fun for anyone, but if I were confronted with someone checking in any of the iterations of this function in this thread to a codebase that I was using, I would just delete and rewrite it.
     
    Last edited: Mar 24, 2014
  20. Mar 25, 2014 #19

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    craigi, while you may see .Scott's solutions as "beautiful", those who advocate early return as the preferred mechanism for dealing with invalid inputs (failed preconditions) will inevitably use a rather different adjective to describe that code. I see what .Scott's wrote as exemplifying why one *should* use early return rather than shun it.


    This is a programming religion issue. As far as I can tell, there are no studies that compares "single point of entry / single point of return" versus early return with regard to readability, understandability, maintainability, etc. There's only religion. From my experience, mandating religious issues rarely works. Those religious mandates cause managers to tell the programmers who work for them to ignore the programming standards. All of them. I've seen this happen, multiple times.

    The "single point of entry / single point of return" rule ranks right up there with regard to causing dissension as do the "no magic numbers" rule (which taken to its extreme results in nonsense such as #define ZERO 0) and the yoda convention rule (which results in inscrutable code such as if (ZERO != number) ...).
     
    Last edited: Mar 25, 2014
  21. Mar 25, 2014 #20
    I feel like an idiot sometimes. I understand that I didn't pass highest as a reference and didn't initialize it inside the function.

    How about this:

    Code (Text):

    template <typename T> T max(T* arr, size_t n)
    {
        if (n == 0)  /* I know most people programmers prefer "if (!n)" */
            throw("The max of an empty array is undefined, bro.");

        /* The rest doesn't need to be in an "else" statement, even though the intent of
            the function is to execute the rest of the code only if the "if" condition isn't met */
        T highest = arr[0];
        for (T* i(arr+1), j(arr+n); i != j; ++i)
        /* Pointer arithmetic starting at the memory address after the first element of the array and
            continuing until one-off-the-end of the array */  
            if (*it > highest)
                  highest = *it;
            return highest;
    }
     
    I'm sure a C++ purist will have some fancier routine that involves forward iterators, recursion, etc.
     
Know someone interested in this topic? Share this thread via Reddit, Google+, Twitter, or Facebook