for loop termination

Discussion in 'C++' started by werasm, Nov 22, 2007.

  1. werasm

    werasm Guest

    Hi all,

    I've been following the discussions concerning loops and
    whether to break or terminate mimicking the condition etc.

    I've had this recent case (which caused a bug) where I
    had to do something on checking the condition, whereafter
    exiting (the loop).

    It went something like this:

    for( Sequence::iterator i = mySeq.begin(); i != mySeq.end(); ++i )
    {
    i->doSomething();

    if( *i->hasCertainCondition() )
    {
    i->doSomethingElse();
    //essentially break out.
    i = mySeq.end(); //Force terminate :)
    }
    }

    All in all the loop was more complicated, but this suffices the
    example.
    This caused a crash (as I recall). <i> incremented after the loop was
    completed whereafter the condition were checked. The increment of <i>
    caused incrementing past the end (causing assertion).

    I suppose there are other ways in which this could be done, but
    <break>
    statement would have solved the problem and would not have caused the
    bug. Just merely (or naively perhaps) trying to be conventional caused
    the
    bug.

    Of course one could have used (i = mySeq().end - 1), but <break>
    seems less error prone to me.

    It could also be noted that the loop is quite a rare case, where you
    want to do something for each item, and if this causes the item
    to change state, you want to do something else and then not
    continue. Other possibilities that sprung to mind were:

    Sequence::iterator i = mySeq.begin();
    for( i; (i != mySeq.end()) && (not i->hasCertainCondition()); ++i )
    {
    i->doSomething();
    }
    //if loop terminated early, doSomethingElse
    if( i != mySeq.end() )
    {
    i->doSomethingElse();
    }

    Using the original loop with a break seemed the cleanest to me. I've
    noticed some "perhaps respected" posters shuns this approach.
    What would you in general consider "the cleaner" solution?

    One that I certainly don't consider cleaner is forcing the
    termination
    condition (as this is more error prone than break).

    Regards,

    Werner
    werasm, Nov 22, 2007
    #1
    1. Advertising

  2. werasm

    Kai-Uwe Bux Guest

    werasm wrote:

    > Hi all,
    >
    > I've been following the discussions concerning loops and
    > whether to break or terminate mimicking the condition etc.
    >
    > I've had this recent case (which caused a bug) where I
    > had to do something on checking the condition, whereafter
    > exiting (the loop).
    >
    > It went something like this:
    >
    > for( Sequence::iterator i = mySeq.begin(); i != mySeq.end(); ++i )
    > {
    > i->doSomething();
    >
    > if( *i->hasCertainCondition() )
    > {
    > i->doSomethingElse();
    > //essentially break out.
    > i = mySeq.end(); //Force terminate :)
    > }
    > }
    >
    > All in all the loop was more complicated, but this suffices the
    > example.
    > This caused a crash (as I recall). <i> incremented after the loop was
    > completed whereafter the condition were checked. The increment of <i>
    > caused incrementing past the end (causing assertion).
    >
    > I suppose there are other ways in which this could be done, but
    > <break>
    > statement would have solved the problem and would not have caused the
    > bug. Just merely (or naively perhaps) trying to be conventional caused
    > the
    > bug.
    >
    > Of course one could have used (i = mySeq().end - 1), but <break>
    > seems less error prone to me.
    >
    > It could also be noted that the loop is quite a rare case, where you
    > want to do something for each item, and if this causes the item
    > to change state, you want to do something else and then not
    > continue. Other possibilities that sprung to mind were:
    >
    > Sequence::iterator i = mySeq.begin();
    > for( i; (i != mySeq.end()) && (not i->hasCertainCondition()); ++i )
    > {
    > i->doSomething();
    > }
    > //if loop terminated early, doSomethingElse
    > if( i != mySeq.end() )
    > {
    > i->doSomethingElse();
    > }


    This version is not equivalent to the first version (with the break-fix).
    The different lies in the relative ordering of

    i->doSomething() vs i->hasCertainCondition()



    > Using the original loop with a break seemed the cleanest to me. I've
    > noticed some "perhaps respected" posters shuns this approach.
    > What would you in general consider "the cleaner" solution?


    Don't know. But here are some alternatives:

    a) The (hopefully) "correct" version of the i = mySeq.end() trick:

    Sequence::iterator i = mySeq.begin();
    whlie ( i != mySeq.end() ) {
    i->doSomething();
    if ( i->hasCertainCondition() ) {
    i->doSomethingElse();
    i = mySeq.end();
    } else {
    ++i;
    }
    }

    b) The break-loop

    Sequence::iterator i = mySet.begin();
    for ( ; i != mySeq.end(); ++i ) {
    i->doSomething();
    if ( i->hasCertainCondition() ) {
    i->doSomethingElse();
    break;
    }
    }

    The main difference between the two approaches is the post-loop value of i.
    In the second case, you can tell which way the loop ended. This leads to

    c) Pulling i->doSomethingElse() out:

    Sequence::iterator i = mySeq.begin();
    for( ; i != mySeq.end(); ++i ) {
    i -> doSomething();
    if ( i->hasCertainCondition() ) {
    break;
    }
    }
    if ( i != mySeq.end() ) {
    i->doSomethingElse();
    }

    This opens up the following option:

    d) Reverse the loop-conditions:

    Sequence::iterator i = mySeq.begin();
    do {
    if ( i == mySeq.end() ) {
    break;
    }
    i->doSomething();
    } while ( i->hasCertainCondition(), ++i );
    if ( i != mySeq.end() ) {
    i->doSomethingElse();
    }

    And finally:

    e) No break:

    Sequence::iterator i = mySeq.begin();
    if ( i != mySeq.end() ) {
    do {
    i->doSomething();
    } while ( i->hasCertainCondition() && ++i != mySeq.end() );
    if ( i != mySeq.end() ) {
    i->doSomethingElse();
    }
    }


    Make your pick. The point is that there is some inherent complexity in a
    loop with two exit points and all you can do is move it around. You cannot
    eliminate it.


    > One that I certainly don't consider cleaner is forcing the
    > termination
    > condition (as this is more error prone than break).


    Huh? I do not really understand what you mean. Could you demonstrate this
    approach in code?


    Best

    Kai-Uwe Bux
    Kai-Uwe Bux, Nov 22, 2007
    #2
    1. Advertising

  3. werasm

    werasm Guest

    On Nov 22, 12:44 pm, Kai-Uwe Bux <> wrote:
    > > Sequence::iterator i = mySeq.begin();
    > > for( i; (i != mySeq.end()) && (not i->hasCertainCondition()); ++i )
    > > {
    > > i->doSomething();
    > > }
    > > //if loop terminated early, doSomethingElse
    > > if( i != mySeq.end() )
    > > {
    > > i->doSomethingElse();
    > > }

    >
    > This version is not equivalent to the first version (with the break-fix).
    > The different lies in the relative ordering of
    >
    > i->doSomething() vs i->hasCertainCondition()


    Oops! Yes, you are right. Ugly mess.

    > > Using the original loop with a break seemed the cleanest to me. I've
    > > noticed some "perhaps respected" posters shuns this approach.
    > > What would you in general consider "the cleaner" solution?

    >
    > Don't know. But here are some alternatives:
    >
    > a) The (hopefully) "correct" version of the i = mySeq.end() trick:
    >
    > Sequence::iterator i = mySeq.begin();
    > while ( i != mySeq.end() ) {
    > i->doSomething();
    > if ( i->hasCertainCondition() ) {
    > i->doSomethingElse();
    > i = mySeq.end();
    > } else {
    > ++i;
    > }
    > }


    Yes. That implies not using a for at all. OK. Nice solution.

    > b) The break-loop
    >
    > Sequence::iterator i = mySet.begin();
    > for ( ; i != mySeq.end(); ++i ) {
    > i->doSomething();
    > if ( i->hasCertainCondition() ) {
    > i->doSomethingElse();
    > break;
    > }
    > }


    This was exactly what I had in mind, but this is what
    some consider "bad practice" - breaking out of a for.

    > d) Reverse the loop-conditions:
    >
    > Sequence::iterator i = mySeq.begin();
    > do {
    > if ( i == mySeq.end() ) {
    > break;
    > }
    > i->doSomething();
    > } while ( i->hasCertainCondition(), ++i );
    > if ( i != mySeq.end() ) {
    > i->doSomethingElse();
    > }


    Ugly.

    > e) No break:
    >
    > Sequence::iterator i = mySeq.begin();
    > if ( i != mySeq.end() ) {
    > do {
    > i->doSomething();
    > } while ( i->hasCertainCondition() && ++i != mySeq.end() );
    > if ( i != mySeq.end() ) {
    > i->doSomethingElse();
    > }
    > }


    Hmmm. I tend to never use whiles and do-whiles as alternative.
    I must say that for me the least error prone is simply
    breaking (solution b). Solution (e) only has one exit point,
    but it is tedious.

    Probably begs the question:

    Should we write tedious code merely to perfect the art of
    having one exit point? To me the for/break solution is
    much clearer.

    > > One that I certainly don't consider cleaner is forcing the
    > > termination
    > > condition (as this is more error prone than break).

    >
    > Huh? I do not really understand what you mean. Could you demonstrate this
    > approach in code?


    My first approach attempted forced the termination of the for
    by setting the iterator to end() [end()-1 would have worked].
    For me this was forcing the termination condition, which means
    that I might of well just have called "break".

    Thanks for your response.

    Werner
    werasm, Nov 22, 2007
    #3
  4. werasm

    Kira Yamato Guest

    On 2007-11-22 07:27:14 -0500, werasm <> said:

    >
    > Probably begs the question:
    >
    > Should we write tedious code merely to perfect the art of
    > having one exit point? To me the for/break solution is
    > much clearer.


    I like it the way the official FAQ
    http://www.parashift.com/c -faq-lite/ puts it:

    "Every interface you build has a cost and a benefit. Every reusable
    component you build has a cost and a benefit. Every test case, every
    cleanly structured thing-a-ma-bob, every investment of any sort. You
    should never invest any time or any money in any thing if there is not
    a positive return on that investment. If it costs your company more
    than it saves, don't do it!"

    So the question you have to ask yourself is this: What does the
    perfection of the art cost you? What does it buy you?

    To me, the best code is no code. What I mean here is the least amount
    of code is almost always the most readable and maintainable code. Keep
    it simple and straight forward.

    If a simple break statement will do the trick and its use is clear and
    obvious, then use it.

    If you're nested deeply in many loops and you've reached a terminating
    condition, then use a goto to jump out of all the loops. Quit messing
    with setting up and checking for 'done' flags, or force the program
    flow to plow through every loops' terminating conditions.

    To me, the most important piece of the software is the class public
    interfaces. How the internal implementation is done is much less
    important.

    --

    -kira
    Kira Yamato, Nov 22, 2007
    #4
  5. werasm

    werasm Guest

    On Nov 22, 4:14 pm, Kira Yamato <> wrote:


    > To me, the most important piece of the software is the class public
    > interfaces. How the internal implementation is done is much less
    > important.


    Yes, but I'm also the one that needs to do that internal
    implementation
    correctly, and per chance one day someone will need to do maintenance
    by looking at existing code (implementation). I know that features
    should not be added by modifying existing code (or at least by
    modifying
    as little as possible existing code). Therefore, when some does look
    at the implementation (ever), I would hope that it would be crystal
    clear to that someone what my intent was.

    For this reason (I'm perhaps on your side here;-), I would want
    to know - why not the break or the goto in a deeply nested loop
    (the example being a case where not using break went wrong).

    Kind regards,

    Werner
    werasm, Nov 22, 2007
    #5
  6. werasm

    David Harmon Guest

    On Thu, 22 Nov 2007 07:35:38 -0800 (PST) in comp.lang.c++, werasm
    <> wrote,
    >For this reason (I'm perhaps on your side here;-), I would want
    >to know - why not the break or the goto in a deeply nested loop
    >(the example being a case where not using break went wrong).


    The whole "structured programming" argument about writing loops with
    a single point of exit was about whether or not the resulting code
    was understandable and maintainable. If you can look at the result
    and say "that's simple" you have done right. If you look at it and
    say "okay, that trick will work" then beware.

    In my opinion, the "it=end()" trick is in the latter category, and
    "break" is usually simple. Not more than one "break". Before I
    would use "it=end()" I would use "termination_flag=true".

    For a nested loop, I will often put it in its own function and
    "return" from the middle when the result is found, instead of
    "break". Small functions that do a well-defined thing are worth
    more for comprehensibility than single-exit is.
    David Harmon, Nov 22, 2007
    #6
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. JezB
    Replies:
    3
    Views:
    21,967
  2. Garth17
    Replies:
    0
    Views:
    561
    Garth17
    Mar 16, 2005
  3. Itamar Lev
    Replies:
    4
    Views:
    453
    Itamar Lev
    Sep 4, 2003
  4. grocery_stocker
    Replies:
    6
    Views:
    315
    Tommi Johnsson
    Jul 23, 2005
  5. Isaac Won
    Replies:
    9
    Views:
    371
    Ulrich Eckhardt
    Mar 4, 2013
Loading...

Share This Page