Removing else blocks

Collapse
This topic is closed.
X
X
 
  • Time
  • Show
Clear All
new posts
  • qazmlp

    Removing else blocks

    I have an 'if else' statement block like this:

    if( inputArg != VALID_VALUE )
    {
    return error ;
    }
    else
    {
    // Do some major operation
    return success ;
    }

    I am thinking of changing it to something like this:
    if( inputArg != VALID_VALUE )
    {
    return error ;
    }

    // Do some major operation
    return success ;


    Which style do you recommend? Why?
  • John Harrison

    #2
    Re: Removing else blocks


    "qazmlp" <qazmlp1209@red iffmail.com> wrote in message
    news:db9bbf31.0 403070140.87aef 3d@posting.goog le.com...[color=blue]
    > I have an 'if else' statement block like this:
    >
    > if( inputArg != VALID_VALUE )
    > {
    > return error ;
    > }
    > else
    > {
    > // Do some major operation
    > return success ;
    > }
    >
    > I am thinking of changing it to something like this:
    > if( inputArg != VALID_VALUE )
    > {
    > return error ;
    > }
    >
    > // Do some major operation
    > return success ;
    >
    >
    > Which style do you recommend? Why?[/color]

    The second.

    Because exceptional processing (like error handling) should not be given
    equal weight with normal processing. By having the error handling part of
    you function in a separate if statement from the main part of the function
    you are demonstrating that both pieces of code don't have equal importance.
    Having both parts of the code in an if else statement implies some sort of
    equivalence.

    john


    Comment

    • Thore Karlsen

      #3
      Re: Removing else blocks

      On 7 Mar 2004 01:40:13 -0800, qazmlp1209@redi ffmail.com (qazmlp) wrote:
      [color=blue]
      >I have an 'if else' statement block like this:
      >
      >if( inputArg != VALID_VALUE )
      >{
      > return error ;
      >}
      >else
      >{
      > // Do some major operation
      > return success ;
      >}
      >
      >I am thinking of changing it to something like this:
      >if( inputArg != VALID_VALUE )
      >{
      > return error ;
      >}
      >
      >// Do some major operation
      >return success ;
      >
      >
      >Which style do you recommend? Why?[/color]

      I typically use the second style, because it avoids needless
      indentation. I prefer to bail out immediately if there's a problem,
      instead of having a whole mess with multiple if/elses.

      There are those that think that a function should only have one exit,
      and it's quite a religious issue. Personally, I think that rule just
      causes longer, more unreadable, and more unmaintanable code. I keep my
      functions short and simple, so the single exit rule is not as beneficial
      as perhaps it would be in larger functions.

      --
      Be seeing you.

      Comment

      • John Harrison

        #4
        Re: Removing else blocks

        > There are those that think that a function should only have one exit,[color=blue]
        > and it's quite a religious issue.[/color]

        This is structured programming, as exemplified by Pascal. Every function
        should have one entrance point and one exit point. Everyone accepts the
        first part, very few the second part, especially if they have done real
        world programming where you do have to think about error handling.

        C++ exceptions provide an even clearer way to bail out of a function upon
        detecting an error. In code like this the use of else looks perverse to me.

        if (some error)
        {
        throw some exception;
        }
        else
        {
        carry on as normal;
        }

        john


        Comment

        • Fraser Ross

          #5
          Re: Removing else blocks

          The first style might result in a 'function should return a value' warning
          even though it always does. I would use the second style for that reason.

          Fraesr.


          Comment

          • Jeff Schwab

            #6
            Re: Removing else blocks

            qazmlp wrote:[color=blue]
            > I have an 'if else' statement block like this:
            >
            > if( inputArg != VALID_VALUE )
            > {
            > return error ;
            > }
            > else
            > {
            > // Do some major operation
            > return success ;
            > }
            >
            > I am thinking of changing it to something like this:
            > if( inputArg != VALID_VALUE )
            > {
            > return error ;
            > }
            >
            > // Do some major operation
            > return success ;
            >
            >
            > Which style do you recommend? Why?[/color]

            When I'm checking for something I expect to happen in the normal course
            of the program, my preference is to maintain a single point of exit:

            Status function( Input const& input ) {

            Status status; // To be returned.

            if( ! valid( input ) ) {
            status = error;
            }
            else { // Input is valid.
            status = success;
            }

            return status;
            }

            The biggest reason for this is that it guarantees a place in each
            function where I can check the function's value (and any other program
            state) immediately before the function returns. This isn't something I
            did in school; it took a couple of years of "real world" debugging to
            appreciate.

            If a run-time check uncovers a truly exceptional condition, I prefer to
            throw an exception. "Truly exceptional" means (to me) that it may not
            be feasible to recover from the error. If one of my functions is called
            incorrectly, I typically throw, since I no longer trust the immediately
            higher layer of the program.

            template< typename N >
            N reciprocal( N const& n ) {

            if( n == 0 ) {
            throw Exceptions::Div ision_by_zero(
            "attempt to find reciprocal of zero" );
            }

            N result = 1/n;

            return result;
            }

            In most of these cases I actually use an ASSERT macro to do the "throw
            if check fails." I know Bjarne recommends using a template for
            Assertions, but I like my error messages to include the failed
            expression, as well as the file name and line number.

            As Thore said, this is a pretty religious issue. Bright people have
            suprisingly different philosophies. The conversation seems to be along
            these lines:

            A: I like to keep a single point of exit.
            B: That makes the code less readable.
            A: But much more debuggable.
            B: No, you're wrong.
            A: No, YOU are wrong.
            B: Taste my wrath!

            [ Mild flames ensue. ]

            Whatever you do, you're likely to upset a few folks. Pick whatever
            makes sense to you and go with it. If you change your mind later, at
            least you'll know *why* the way you've been doing it is wrong. Then you
            can be religious about it, too. ;)


            Comment

            • Magnus

              #7
              Re: Removing else blocks


              "Jeff Schwab" <jeffplus@comca st.net> skrev i melding
              news:VMidnbP4NI 0ah9bdRVn-ug@comcast.com. ..[color=blue]
              >
              > When I'm checking for something I expect to happen in the normal course
              > of the program, my preference is to maintain a single point of exit:
              >[/color]
              [snip]

              In certain situations, and maybe in the long run, might not this give you a
              performance penalty, in contrast to exiting the function once an error is
              discovered ?

              - Magnus


              Comment

              Working...