Alternatives to modifying loop var in the loop.

P

Phil Carmody

James Kuyper said:
The validity of that guideline depends upon what you're doing. For
instance, if a loop starts with

for(int item=0; item < item_count; item++)

then it is potentially very confusing to change either item or
item_count within the loop. That doesn't mean you shouldn't do it - if
there's sufficiently good reason so do so, then you should; but you
should draw attention to the lines where you perform the modification by
inserting comments.

Sometimes even variable names can be enough of a clue, but yes, that
single line is enough of an idiom that it carries a lot of baggage about
what one would expect to follow.
On the other hand, I've written code to do a bracketed binary search of
a sorted array like the following:

while(low < high)
{
int mid = (low+high)/2;
if(array[mid] < x)
low = mid+1;
else
high = mid;
}

The fact that this code modifies both "low" and "high" inside the loop
is not a problem, because it's very clear why and how it's modifying them.

I notice that one example was a for() and the other was a while().
I'm a great believer that people over-use for() - I hate seeing ``; )'',
for example. That immediately says "should have been implemented as a
while loop" to me. My expectations are different from the two constructs
certainly.

Phil
 
P

Phil Carmody

Dr Nick said:
I've not checked to see if it's still the case, but one of the most
irritating features of GCC that I've come across is that it warns that
this is not a complete initiator.

Now if I've done = {1,2,3,4,5,6} for a 7 element array, I may well not
want the last initialised to 0, so I can understand the warning. But =
{0} should be a special case.

If we're hypothesising, then why not have the default zero initialiser
syntax be = {}; ? 2 initialisers is some. 1 initialiser is some. 0
initialisers is none. If isolating a special case, surely the one that
stands on its own (none, vs. some) is the best one to choose?
Of course, you can always turn off warnings etc, but this particular one
seemed especially irritating.

I have a few "favourites" which I like less.
Phil
 
S

Seebs

I think that "elegance" means different things to the two
of us (eye of the beholder, again). The quality you describe
as "elegance" is something I'd prefer to call "clarity," and I
agree that it's a desirable attribute regardless of whether you
call it clarigance or eleganity.

Hmm. You may have a point.
... and that's why I don't think "elegance" is a useful
criterion: If you can't measure it, you can't tell whether it's
present or absent, or to what degree.

I'm not sure that's right. Or rather: Even if things are subjective,
they may still be useful criteria. They may be observer-variant, but I
would tend to consider "pleasant to work on" a significant trait to
consider when evaluating prospective projects.
[*] IIRC, Weinberg reported on a similar experiment in "The
Psychology of Computer Programming." Groups of computer science
students read the same piece of code, either in its original form
or with the comments removed. The students who saw the UNcommented
code found and fixed more of its errors than those who had the
"benefit" of the commentary ...

That's not surprising. Comments that tell you what's supposed to happen
are likely to make you expect that to be what happens.

I usually aim comments at explaining why I do something unexpected
that took a while to figure out. Otherwise, code is usually pretty
comprehensible if it's reasonably clear.

-s
 
K

Kenny McCormack

Eric Sosman said:
I think that "elegance" means different things to the two
of us (eye of the beholder, again). The quality you describe
as "elegance" is something I'd prefer to call "clarity," and I
agree that it's a desirable attribute regardless of whether you
call it clarigance or eleganity.

Interestingly enough, the actual, original definition of "elegant" is
"minimalist". Which, in the context of the religion of CLC, ends up
meaning exactly the opposite of "clarity" (since "clarity" usually ends up
meaning "be as verbose as possible").

--
The problem in US politics today is that it is no longer a Right/Left
thing, or a Conservative/Liberal thing, or even a Republican/Democrat
thing, but rather an Insane/not-Insane thing.

(And no, there's no way you can spin this into any confusion about
who's who...)
 
A

Aleksandar Kuktin

Finally, to your "backtracking counter" loop: I really,
really don't like it. A better formulation, I think, would be

for (i = 0; i < len; ) {
if (needs_deleting(arr)
len = del_at(i, len, arr);
else
++i;
}

... because the reader will not be fooled by the familiar-looking `i++'
in the first line and perhaps overlook the unfamiliar in-loop
adjustment. Seeing no index adjustment at all in the `for', he will
look inside the loop to learn what happens to `i', and (I think) his
route to understanding will be a shorter one.


See, that's *exactly* the kind of response I was looking for. It teaches
me an alternative way of doing things I myself had doubts about when
doing them the way I did them.

As to the 'elegance' discussion, I though for a while what word to use
and basically chose 'elegance' over others because I thought it had the
higest chance of conveying the intended meaning and producing the desired
response. The fact I got the response I wished for basically proves my
choice right. :)
 
M

Matt


I plugged the new getscore in to the apple][ code and got a performance
gain. 15min wait on the first pass vs 20min previously.

I think I discovered something else though.

Looking at a single conceptually boolean int to see whether it is
necessary to do something to something seems like a trivial operation.

What if you did it 1296 * (15 * 1296) times and only 1296 * (1 * 1296)
of them were necessary? Could this slow your program down?

If anyone is curious the offending code is the loop at line 178 of
http://code.mattsmith.org.nz/mastermind/aztecc65/main.c

I think Eric was on the money when he said
I suspect something's
seriously amiss with your setup

And I was on the money when I said
I'm pretty sure it'll turn out to be my program

Matt.
 
J

Jorgen Grahn

Matt wrote:
) I'm sure I've read somewhere that it's considered bad practice to modify
) the value of the variable on the right of a comparison operator inside
) the loop concerned.
Perhaps the 'bad practice' you read about was this idiom, which is used to
avoid 'break' statements:

for (i = 0; i < len; i++) {
if (needs_ending(arr)) {
i = len;
} else {
do_something_with(arr);
}
}

This one may prevent or break optimizations based on setting the loop
count prior to entering the loop. The break also would prevent such
optimizations, but the compiler would have no excuse for missing it.

It's also terribly ugly, IMO. "The cure is worse than the disease" is
a phrase which comes to mind. A big part of the appeal of for loops
is "look, here's all you have to know about how 'i' changes, in one
single line".

(I don't think I've seem it before, except of course the form with an
explicit "please stop the loop" flag -- 'i < len && !stop')

/Jorgen
 
E

Eric Sosman


I plugged the new getscore in to the apple][ code and got a performance
gain. 15min wait on the first pass vs 20min previously.

Still seems far too long. See below.
I think I discovered something else though.

Looking at a single conceptually boolean int to see whether it is
necessary to do something to something seems like a trivial operation.

What if you did it 1296 * (15 * 1296) times and only 1296 * (1 * 1296)
of them were necessary? Could this slow your program down?

If you drank fifteen times your normal intake of beer,
could this slow *you* down? ;-)

Might this account for your fifteen minutes? Let's use the
back of this handy envelope here: 25e6 tests divided by 900 seconds
is 28000 tests/sec, or 36 microseconds/test. You've mentioned that
you're using an emulator for an elderly machine, and "36 MHz" might
be in the right ballpark. If the host machine's instruction set is
dissimilar to that of the emulated system, the emulator may well
be an interpreter; such things often execute dozens to hundreds
of host instructions for each emulated instruction. If the test
and branch amounts to four or five emulated instructions, a twenty-
fold dilation would turn "36 MHz" into "3 or so GHz," which would
be something akin to what one would get from a present-day machine.

Of course, this doesn't *prove* the extra work is soaking up
all or even most of your time, but the numbers seem plausible and
do not rule out the possibility. I think you'd do well to give
this matter further attention.
 
J

Jorgen Grahn

.
I notice that one example was a for() and the other was a while().
I'm a great believer that people over-use for() - I hate seeing ``; )'',
for example.

Me too, but I don't seem to see it a lot. My feeling is people abused
for() more in the past, perhaps because the optimizers were worse.
That immediately says "should have been implemented as a
while loop" to me.

Yes. 'for' is IMHO good for a few (but common!) cases only.

/Jorgen
 
J

Jorgen Grahn

Hmm. You may have a point.


I'm not sure that's right. Or rather: Even if things are subjective,
they may still be useful criteria.

Yes. (Also, even the interesting properties you /can/ measure are
usually infeasible to measure. You won't have a team of psychologists
and a study group handy when it's time for code review ...)

Almost all conflicts I have with coworkers, people on Usenet, previous
authors of code I have to maintain, etc ... are about subjective
things. E.g. the code is reasonably correct, but it's not as easy
(pleasant) to work with as I'd like it.

Frustratingly, too often (most of the time, it seems) what's readable
to one person is not very readable to all others. Only the people who
just don't care are unaffected.

Clarity as a concept doesn't seem a lot more helpful. Your background
and way of thinking has too much of an impact on what you see as
"clear".

/Jorgen
 
E

Eric Sosman

On 28/12/13 20:19, Ben Bacarisse wrote:
you have a redundant loop in
getscore which is the time-limiting function right now. You can count
the matching colours without looping over colours.

http://code.mattsmith.org.nz/mastermind/bits/new_get_score.c

I plugged the new getscore in to the apple][ code and got a performance
gain. 15min wait on the first pass vs 20min previously.

Still seems far too long. See below.
I think I discovered something else though.

Looking at a single conceptually boolean int to see whether it is
necessary to do something to something seems like a trivial operation.

What if you did it 1296 * (15 * 1296) times and only 1296 * (1 * 1296)
of them were necessary? Could this slow your program down?

If you drank fifteen times your normal intake of beer,
could this slow *you* down? ;-)

Might this account for your fifteen minutes? Let's use the
back of this handy envelope here: 25e6 tests divided by 900 seconds
is 28000 tests/sec, or 36 microseconds/test. You've mentioned that
you're using an emulator for an elderly machine, and "36 MHz" might
be in the right ballpark. [...]

Botch. Botch, botch, botchety-botch-botch-botch.

"Hold it right there, fella. Put down the pencil, and step
away from the envelope. Okay, let's see your estimating license.
No, take it out of your wallet first, that's right. Well, well,
what have we here? Estimating with a suspended license? Don't
explain to me; save it for the judge. You have the right to
remain silent, ..."

"Honestly, Your Honor, I don't know what came over me. The
cat was trying to get my attention and maybe that distracted me,
but I still don't know why I pushed one-over-ex after calculating
a rate, and then called *that* a rate, and mangled the units, too.
28000 tests/sec has nothing to do with `36 MHz' -- it's 28 kHz or
0.028 MHz, as any fool can plainly see. No offense, Your Honor.
And all that stuff about a hundredfold slowdown explaining the
difference between `36 MHz' and a modern machine -- well, it's
completely off the mark, I see that now. It'd take something like
a hundred-thousand-fold slowdown to cover the actual discrepancy,
and that's more than I'd like to believe in. It'd still be good
for him to figure out why he's doing fifteen times as much work as
necessary, but that in itself won't account for his time."

"No, Your Honor, I don't hold a grudge against him -- I don't
even know him. I certainly wasn't trying to make trouble for him,
I just blundered, that's all. Nothing malicious, just a thinko."

"Uh, no, it's not my first offense. My worst, maybe, but not
my first. How many? Well, in four-plus decades of estimating I
must have-- Oh, right, sorry: With a suspended license, I shouldn't
be trying to estimate how many estimations I've mis-estimated. All
I can say is that there was no ill will, and that I'll be more
careful in the future, and I plead for the Court's understanding."
 
G

glen herrmannsfeldt

Eric Sosman said:
On 12/30/2013 6:11 PM, Eric Sosman wrote: (snip)
(snip)

Botch. Botch, botch, botchety-botch-botch-botch.
"Hold it right there, fella. Put down the pencil, and step
away from the envelope. Okay, let's see your estimating license.
No, take it out of your wallet first, that's right. Well, well,
what have we here? Estimating with a suspended license? Don't
explain to me; save it for the judge. You have the right to
remain silent, ..."
"Honestly, Your Honor, I don't know what came over me. The
cat was trying to get my attention and maybe that distracted me,
but I still don't know why I pushed one-over-ex after calculating
a rate, and then called *that* a rate, and mangled the units, too.
28000 tests/sec has nothing to do with `36 MHz' -- it's 28 kHz or
0.028 MHz, as any fool can plainly see.

Reminds me how often 1/x is misapplied. Resolution should be in
spatial frequency (something per unit distance) not distance
units. Otherwise "high resolution" has the wrong meaning.

More intersting is shutter speed on cameras, which should be
in reciprocal time units. That is, not (1/125) second, but
125/second. (Which nicely agrees with the labels on the
shutter speed dial, but not with the common description.)
Again, high shutter speed needs to have the right meaning.

Yes, the one-over-ex key gets pressed too often.
No offense, Your Honor.
And all that stuff about a hundredfold slowdown explaining the
difference between `36 MHz' and a modern machine -- well, it's
completely off the mark, I see that now. It'd take something like
a hundred-thousand-fold slowdown to cover the actual discrepancy,
and that's more than I'd like to believe in. It'd still be good
for him to figure out why he's doing fifteen times as much work as
necessary, but that in itself won't account for his time."

(snip, really funny, too)

-- glen
 
B

Ben Bacarisse

glen herrmannsfeldt said:
More intersting is shutter speed on cameras, which should be
in reciprocal time units. That is, not (1/125) second, but
125/second. (Which nicely agrees with the labels on the
shutter speed dial, but not with the common description.)
Again, high shutter speed needs to have the right meaning.

It does. I think you are confusing exposure time with shutter speed. A
high shutter speed is exactly that -- a shutter moving very fast, giving
rise to a very short exposure time.

<snip>
 
G

glen herrmannsfeldt

(snip, I wrote)
It does. I think you are confusing exposure time with shutter speed.
A high shutter speed is exactly that -- a shutter moving very fast,
giving rise to a very short exposure time.

As with the post I replied to, some people put 1/x where it isn't
needed or wanted.

Often people will write shutter speed = (value with time units),
where it comes out right for (value with reciprocal time units.)

-- glen
 
M

Matt

It'd still be good
for him to figure out why he's doing fifteen times as much work as
necessary, but that in itself won't account for his time."

Wrapping up my participation in this thread:

Because constantly rejigging my gcc code for the old compiler was
tedious I ended up using the time taken for the gcc code to solve all
1296 possible codes as a proxy for speed/efficiency/whatever, assuming
any gains made would also manifest when I later rewrote for the 6502
cross compiler.

At one point I got a dramatic improvement:
$ cat *w/time
[...]01//a working version of the code
1295 K3

real 20m43.325s
user 20m43.142s
sys 0m0.048s

[...]05//the next working version (02, 03, 04 all broken or still born)
1295 K3

real 7m13.622s
user 7m13.563s
sys 0m0.004s

Because I would get fed up and start a wholesale rewrite in a new
directory it's a little difficult to pin down exactly what was
responsible for the improvement but here are some changes between the
versions:

1. getscore function rewritten per Ben's advice.
2. I stole Willem's loop (I really like that loop!). This had the effect
of fixing the 15 times too much work thing I'm pretty sure.
3. Fewer functions.
4. Fewer structs appearing in parameter lists. More use of indexes then
looking up the code later.

But as for the single silver bullet I'd have to go back over it all to
find out for sure.

The improvement didn't really manifest in the 6502 version. Or it did
but only after the first pass through the algorithm which still takes 13
min. Weird. I've taken this over to comp.sys.apple2.programmer

So I learned some things:

Thing: If you break your code into lots of tiny functions you can end up
seeing too many trees and not enough of the woods.

Thing: If every time you change your code, you break it then maybe you
had a tenuous or incomplete understanding of the algorithm to begin with.

Thing: Modern implementations like gcc are a huge advance on what was
available for the Apple //e in 1986.

Thanks to all responders,

Matt.
 
K

Kenny McCormack

It depends if you know C or not.

One moron I had the displeasure of working with decided that something
like

while(*d++=*s++);

was not "clear" and needed to be expanded with long ariable names and to
use array notation and an exploded loop.

I pointed out that any C programmer that didn't understand that code
had no right being in the team since its "bread ad butter" for any half
decent C programmer.

I'd be willing to assert that for most working C programmers today, the
above code is (pick all that apply): strange, cryptic, weird, "clever", not
"clear", etc, etc, and that an explicit form with a loop would be much
better in their eyes.

Because, as is true of just about every population, most working C
programmers today are idiots. It's what managememnt wants.
 
A

Aleksandar Kuktin

while(*d++=*s++);

Excuse me, but.... how is this loop supposed to stop. Exactly?


I assume it was meant to be something more akin to

while (*d++ == *s++);

but that the other `=' got lost somewhere.
 
M

Martin Shobe

Excuse me, but.... how is this loop supposed to stop. Exactly?

It stops when *d++ = *s++ is equal to zero. In other words, when s
points to a zero value. It does copy the zero.

Martin Shobe
 
J

James Kuyper

Excuse me, but.... how is this loop supposed to stop. Exactly?

This is something I've always considered to be one of the basic idioms
of C, which is discussed in some detail in K&R. I understood that
discussion on first reading. Over several decades, I've learned that I
must be fairly unusual in that regard - most of the C programmers I've
challenged to provide a detailed explanation of how that construct works
were unable to do so, so don't feel too embarrassed about not having
understood it.

That construct presents several tricky issues, starting with the
question of which gets evaluated first, the * or the ++. The next issue
is precisely which value is returned by the ++ expression. The next
issue is what the value of an assignment expression is. Once you've
thought through those issues, you should be ready to answer the key
question: under what circumstances is the value of the expression
*d++=*s++ equal to zero? Those are precisely the circumstances that will
cause the while loop to terminate.
 
G

Geoff

It stops when *d++ = *s++ is equal to zero. In other words, when s
points to a zero value. It does copy the zero.

Martin Shobe

And here we have the lesson in why the code, while correct and
succinct, is easily misunderstood to be "strange". One has to stop and
say, "er, what?".
 

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. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
474,075
Messages
2,570,549
Members
47,197
Latest member
NDTShavonn

Latest Threads

Top