for loop termination

[werasm]

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

 

[Kai-Uwe Bux]

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

 

[werasm]

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.

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.

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

 

[Kira Yamato]

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.

 

[werasm]

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

 

[David Harmon]

On Thu, 22 Nov 2007 07:35:38 -0800 (PST) in comp.lang.c++, werasm

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.