return in void functions

Discussion in 'C++' started by Arthur Gold, Jan 1, 2004.

  1. Arthur Gold

    Arthur Gold Guest

    Some will quibble about that stylistically (preferring a single exit
    point), but there's nothing wrong with that technique.

    [Of course, you could just as easily have written:

    void SetMapLayer()
    {
    if (Map) layer = LAYER_MAP;
    }

    ]

    HTH,
    --ag
     
    Arthur Gold, Jan 1, 2004
    #1
    1. Advertisements

  2. Arthur Gold

    Ron Natalie Guest

    In structured code, you'd want to avoid willy nilly returning from inside functions,
    but in this case it probably doesn't make any difference. The scary ones are
    things like:

    void SomeFunc() {
    if(...) {
    while( ... ) {
    if(...) {
    try {
    if(...) return
    ...
     
    Ron Natalie, Jan 1, 2004
    #2
    1. Advertisements

  3. Arthur Gold

    Cy Edmunds Guest

    I don't like your construction for two reasons:

    1) It breaks the structured programming "rule" that each function have only
    one return point. I put the word "rule" in quotes because I break it myself
    anytime I feel like it. But here there is no reason to.

    2) It is effectively a double negative. We set layer to LAYER_MAP if !!Map.

    Don't you think this is much clearer?

    void SetMapLayer()
    {
    if (Map)
    layer = LAYER_MAP;
    }

    One return point, no double negatives.

    Getting back to your original question, I would use return in a void
    function if I thought the net effect was to make the code simpler. Example:

    void fribble(int thing)
    {
    if (thing == 0)
    return; // explanation of why here
    // 15 lines of code here
    }

    The structured programming version is

    void fribble(int thing)
    {
    if (thing != 0)
    {
    // 15 lines of code here
    }
    }

    which I think is somewhat less readable, although by no means a serious
    mistake.
     
    Cy Edmunds, Jan 1, 2004
    #3
  4. Arthur Gold

    Mike Wahler Guest

    This is a matter of 'taste' or 'style'. I prefer to have a
    function have only one exit point (not always possible, but
    I strive for it.)

    I'd write the above as:

    void SetMapLayer()
    {
    if(Map)
    layer = LAYER_MAP;
    }

    which effectively does the same thing.

    Also, even if I'd written it as you have, I'd put the 'return'
    statement on a separate line:

    if( !Map )
    return;

    layer = LAYER_MAP;

    This makes it more quickly obvious (to me at least) that
    the 'linear' execution flow is interrupted. I'd probably
    also add a comment for more quick visual identification:

    if( !Map )
    return; /* EXIT POINT */
    /* I also like a blank line here */
    layer = LAYER_MAP;

    Especially if more code is added after the assignment, this
    makes the 'return' stand out.

    Again, this is all a matter of 'style'. IMO more important that
    the actual 'style' you use, is that you use it consistently.

    -Mike
     
    Mike Wahler, Jan 1, 2004
    #4
  5. Arthur Gold

    Jeff Schwab Guest

    It's not just a matter of 'style.' Every extra point of return makes
    code much harder to debug, since state-validation code to be executed
    before a function returns must be duplicated for each point of return.
    Take this advice from someone who has learned the error of his ways:
    Keep it down to one exit point whenever possible. The only exception to
    this rule should be the throwing of exceptions.

    Some languages, like Tcl, do have great support for techniques relying
    on multiple points of return. C++ is not such a language.

    -Jeff
     
    Jeff Schwab, Jan 1, 2004
    #5
  6. Arthur Gold

    Maximus Guest

    Hi,
    I was just wondering, is it good to use return without arguments in a void
    function as following:

    void SetMapLayer()
    {
    if( !Map ) return;
    layer = LAYER_MAP;
    }

    It works well but it is considered a good programming technic?

    TIA, Max.
     
    Maximus, Jan 1, 2004
    #6
  7. Arthur Gold

    Andrew Guest

    ok, help me out here folks, cause I don't quite know how to reply just to
    this comment, but here's mine

    function which take on the form:
    if( !condition ) // did caller do something stupid
    {
    print error
    return
    }
    else
    {
    lots of code
    }

    are to be avoided as I've learned. The reason: runtime inefficiency due to
    cpu branch prediction behavior.
    If I remember correctly, the cpu will pipeline the code which errors and
    exits instead of the typical function
    code resulting is a few holes in the pipeline when it has to skip the error
    and exit code, but worse - all of the
    typical function code will have go into the pipeline "late". I might
    suggest either restructing the if statement or
    ensuring that your compiler fixes it for you and generates optimized object
    code.

    Please do correct me if I am wrong, I hate storing incorrect knowledge in my
    head.

    Andrew
     
    Andrew, Jan 2, 2004
    #7
  8. Arthur Gold

    Clark Cox Guest

    While that may be true on whatever platform you learned on, different
    CPU's predict branches differently.But this is completely outside the
    scope of the C++ language. Besides, since the compiler is free to
    optimize as it sees fit, as long as it doesn't change the actual meaning
    of the code, it could very well reverse the order of the "if {...}" and
    "else {...}" sections of code upon seeing that the condition is "not"-ed.
    You may be right for a particular platform, but you are most
    assuredly wrong for many of the platforms out there.
     
    Clark Cox, Jan 2, 2004
    #8
  9. Arthur Gold

    Chris Dams Guest

    Hello,

    This does not sound very severe. It is a matter of seconds to search for
    every occurrence of the word "return" in a function and insert a common
    line before that. I think conceptually code like

    if(whatever)
    return;

    is very clear and clean. It just means "Do not apply the rest of the
    function if condition _whatever_ is met".

    Have a good 2004,
    Chris Dams
     
    Chris Dams, Jan 2, 2004
    #9
  10. Actually, C++ has a very nice feature for this - the deterministic execution
    of destructors. This one feature is also the reason that the "one exit rule"
    in C++ does not have to be taken to serious. In other languages, I would be
    far more careful to have only one exit point.

    Kind regards
    Peter
     
    Peter Koch Larsen, Jan 2, 2004
    #10
  11. Arthur Gold

    Jeff Schwab Guest

    Destructors don't have access to all of the autmatic state inside the
    function. I suppose you could define all variables at the top of the
    function and pass references into the constructor of a local
    "print-on-destruction" object, but that would be a major pain in the neck.

    This is *not* a minor problem. I have wasted many hours because of it.
     
    Jeff Schwab, Jan 2, 2004
    #11
  12. I'm not following you here, care to explain? I sense design problems, but
    I'm not sure.

    M4
     
    Martijn Lievaart, Jan 2, 2004
    #12
  13. Arthur Gold

    Jeff Schwab Guest

    Why yes, Omniscient One! The design problem was the use of blocks of
    code having multiple exit points. Most of my typical debug cycle is
    spent putting "cerr" lines (or gdb break points) at strategic places in
    the code. Tripling the number of exit points per function almost
    triples the time it takes to debug a problem.

    When I throw an exception in a non-trivial program, I include the file
    name and line number as part of the exception, so I know exactly where I
    need to start looking when an exception is thrown. Trying to track an
    issue back to root cause is far more difficult in spaghetti code than in
    code that avoids mid-block return statements. This should not be news
    to you.

    -Jeff
     
    Jeff Schwab, Jan 2, 2004
    #13
  14. Arthur Gold

    Jorge Rivera Guest

    I guess that after all the crap you got from everybody, by now you know
    that:

    1. It is perfectly valid to use 'return;' in a void function.
    2. You piss a lot of people by doing it.

    :)
     
    Jorge Rivera, Jan 3, 2004
    #14
  15. An easier way to piss a lot of people off is by top posting
     
    Thomas Wintschel, Jan 3, 2004
    #15
  16. Map& Map::SetLayer(void) {
    if (this->Map)
    this->layer = LAYER_MAP;
    return *this;
    }

    Avoid void functions.
    A function should *always* return a value
    so that you can use it in an expression.
    In this case, simply return a [non const] reference to the Map object.

    Never write functions which modify global variables.
    I'll assume that you are implementing a member function here
    and that you reference data member Map to decide whether or not
    to set data member layer to the value of constant LAYER_MAP.

    Instead of using a "setter" function, you might try

    class Map {
    public:
    class Layer_t {
    // . . .
    Layer_t& operator=(const Layer_t&) {
    // . . .
    }
    };
    private:
    // representation
    static const
    Layer_t LAYER_MAP;
    Layer_t layer;
    bool Map;
    public:
    // functions
    const
    Layer_t& Layer(void) const {
    return layer;
    }
    Layer_t& Layer(void) {
    return layer;
    }
    // . . .
    };

    Then, you can write:

    Map m, map;
    // . . .
    m.Layer() = map.Layer();

    for example.
     
    E. Robert Tisdale, Jan 3, 2004
    #16
  17. Arthur Gold

    Maximus Guest

    Well as I can see it is not in all cases, I learned that within a small
    block of codes return can be used but not in a larger one, since it gives
    more than one exit point, thus harder to debug. Thanks for that verry
    pleasent post though.

    Max.
     
    Maximus, Jan 3, 2004
    #17
  18. You'll have a hard time tracing exceptions with this design. Why not
    define a trace object, that prints in constructor and destructor? Much
    easier to use and works with multiple returns and exceptions. It's what I
    do if I need this.

    I never was a fan of not allowing multiple exit points, but exceptions
    embedded it in the language, so I guess we better get used to it. I know I
    don't have any problem with it.

    Obviously, multiple exit points, like any language feature can be used
    and mis-used. I concur that they can introduce problems, but only if
    mis-used. My personal guideline in this is that the code should read like
    a story. If not, you have a maintenance problem at least. So this is not
    really related to multiple exit points, but to obfuscated coding.

    M4
     
    Martijn Lievaart, Jan 3, 2004
    #18
  19. Arthur Gold

    Jeff Schwab Guest

    So, each time you want to debug a function with multiple return points,
    you must:

    1) Define a class with member references to all of the function's local
    variables; at least, the ones you want to trace.

    2) Find a cozy place somewhere before any of the function's return
    points, but after all the traced variables have been defined. IME, this
    is often impossible. Here is a simplistic case that reflects the common
    practice of defining some variable, returning if a check succeeds, and
    otherwise defining some other variables:

    template< typename T >
    void sort_two_objects( T a, T b )
    {
    bool already_sorted = a < b;

    // A Trace object defined here cannot trace the value of copy_of_a,
    // even if the return statement is not used.

    if( already_sorted )
    {
    return;
    }

    // A Trace object defined here will not be destructed if the return
    // statement is used.

    T copy_of_a = a;

    a = b;
    b = copy_of_a;
    }
    Exceptions do provide "extra" exit points, but they are not the same as
    return statements. Exceptions carry information back up the call stack
    about what went wrong, and why. Often, the information obtained in a
    high-level catch block is sufficient, and there is no need to look at
    the lower-level code at all. Even when there is such a need, since my
    exceptions always include the line number and file number of the
    corresponding throw statement, I know exactly which exit point to watch.
    I've heard that theory before. The fact is that programs are not
    stories. When I'm debugging a large piece of code, I have no desire to
    start at the beginning and proceed to the point of the problem. The
    time required for such an approcach is linear with the number of lines
    leading to the point of the problem. By doing instead a binary search
    for the problem, debug time is significantly reduced. This approach is
    eased greatly by the absence of spurious return statements. Any extra
    return statement is what I would call "obfuscated code."

    -Jeff
     
    Jeff Schwab, Jan 3, 2004
    #19
  20. No, use one trace class to trace the program flow. When I want to log
    variables, I use a different (though related) class.
    see below
    LOG("copy_of_a=" << a);
    Why do it any other way? Yes multiple exit points can make ad-hoc
    cerr-debugging harder, but one should not do that anyhow. Writing the
    TRACE and LOG macros is pretty trivial and make life much easier.
    That is true, but I don't see what the problem with multiple exit points
    is if you're going to rerun the program under the debugger anyhow.

    The only real advantage I can see of avoiding multiple exitpoints is that
    one can put a breakpoint on the common exit. In practice, I don't miss it.
    I'm not talking about complete programs being stories, I'm talking about
    pieces of code, most probably a function. If you do that, the complete
    program is easy to read and debugging gets much easier. So your argument
    about binary search is not relevant, that is how one should always debug.

    If the program is hard to read, it is hard to debug. If multiple exit
    points make a function harder to read, they are misused. I still don't see
    why multiple exit points would be a problem here.

    M4
     
    Martijn Lievaart, Jan 3, 2004
    #20
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.