Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Bug/Gross InEfficiency in HeathField's fgetline program

Reply
Thread Tools

Bug/Gross InEfficiency in HeathField's fgetline program

 
 
Richard
Guest
Posts: n/a
 
      10-09-2007
http://www.velocityreviews.com/forums/(E-Mail Removed) (Richard Bos) writes:

> Ian Collins <(E-Mail Removed)> wrote:
>
>> Antoninus Twink wrote:
>> > On 7 Oct 2007 at 22:55, Richard Heathfield wrote:
>> >> Antoninus Twink said:
>> >>> It is also written in a hard-to-read and clunky style.
>> >> A matter of opinion. Which bit did you find hard to read?
>> >
>> > The function is a completely trivial one, yet I can't see it all at once
>> > in my editor without scrolling! Whitespace can help readability, but
>> > excessive whitespace can reduce it, and at the same time give too much
>> > weight to things that aren't important.
>> >

>> You must have a very small screen.

>
> I don't think it's his _screen_ that's very small.
>
> Richard


It is perfectly normal in many cases to debug on smaller displays over
remote lines or to debug in a small window or gdb buffer. The point
remains - smaller footprint is easier to read and follow.

3 clear lines is better than 20 clear lines any day of the week. Long
winded overly whitespaced code is bad.
 
Reply With Quote
 
 
 
 
kuyper
Guest
Posts: n/a
 
      10-09-2007
Richard wrote:
....
> I am astonished that people claiming to be professional programmers
> could be in any way "confused" or "unsure" about your concise
> replacement for Heathfield's version.
>
> ,----
> | while(*u++=(*s=='.' ? '_' : *s))
> | s++;
> `----
>
> I simply can not see the complication or the need to "analyse" this. Yes
> if we were training in lesson 2 we might expand it out a little. but
> only to the extent of removing the ?: usage to an if then else.


I have interviewed several dozen people for C programming positions
over the past 15 years. I've given every single one of them a simple
program to understand and explain and suggest improvements for. The
heart of that program was the following loop:

while(*p++ = *q++);

Only about half of them could even tell me correctly what that loop
does; not a single one has ever correctly explained how it does it.
The only person I've found who ever passed this test wasn't an
applicant but a co-worker, and she required a lot of hints before she
figured it out. The stumbling block for most of them is explaining how
the loop terminates. I've had to hire people who couldn't answer that
question, because better candidates weren't available. One of the most
recent applicants had 15 years(!) of recent C programming experience.
He didn't get the position, but not because of his lack of programming
skills, but only because he lacked several of the other important
qualifications for the position.

You may have had the privilege of working with more competent C
programmers than I have; but I would recommend not overestimating the
competence of the average C programmer. The code you cite has every
feature that made my test difficult for applicants, and adds to that a
use of the ?: operator.

While I can understand your code, even for a use-once-and-throw-away
program, I consider it excessively terse. For improved debugging, I'd
have used an if() statement rather than a ?: operator, so the if-
clause and the else-clause would occur on different lines that could
be selected by the debugger. Any decent compiler should produce the
same code, whether using if() or ?:. Yes, I do write my even use-once
programs anticipating that they may need to be debugged; they tend to
be written in a hurry without review, which means they're much more
likely to be buggy than my longer-lived code.

 
Reply With Quote
 
 
 
 
J. J. Farrell
Guest
Posts: n/a
 
      10-09-2007
On Oct 9, 2:25 pm, Richard <(E-Mail Removed)> wrote:
> Mark McIntyre <(E-Mail Removed)> writes:
> > On Tue, 9 Oct 2007 14:20:41 +0200 (CEST), in comp.lang.c , Antoninus
> > Twink <(E-Mail Removed)> wrote:

>
> >>On 9 Oct 2007 at 9:34, Richard Heathfield wrote:
> >>> Antoninus Twink said:
> >>>> If making the code harder to read in exchange for a small increase in
> >>>> speed is bad,

>
> >>> Whether it is bad depends on the relative merits of performance and
> >>> clarity. I have already shown how I would have to use the program 24/7 for
> >>> almost half a century before your suggested change could save me so much
> >>> as a nanosecond (once the time cost of making that change is factored in),
> >>> and quite frankly I have better things to do with my life. So in this
> >>> case, the performance increase is meaningless, whereas the loss of clarity
> >>> is significant.

>
> >>But exactly the opposite is true - clarity is lost in *your* version,

>
> > You're wrong, but I don't expect you to believe me since you're
> > apparently a troll.

>
> He most certainly is not.
>
> The shorter more concise version would have got through any code review
> long before the long winded one on any project I have worked on.


I can only conclude that you've worked exclusively on very strange
projects. The opposite of what you say is true in my experience. The
original code would always have got through, after questions about the
multiple passes; in the given circumstances the code would be allowed
to stand. Antoninus Twink's version would never have got through a
review. Your version would probably have got through after a lot of
arguing about its being needlessly obscure.

I find your position very surprising. There's no way to know but I'd
bet that, given the choice between the two in the given circumstances,
the vast majority of experienced professional C programmers would
choose the original version over Antoninus Twink's. They might not be
overly fond of it, but it would be the preferred choice of the two.

> >>And if you make simple things over-complicated, we might not
> >>unreasonably suspect that you might make complicated things into a
> >>complete mess.

>
> > Only if he changes his name to Antoninus Twink.

>
> This is getting ridiculous.


Indeed.

 
Reply With Quote
 
santosh
Guest
Posts: n/a
 
      10-09-2007
kuyper wrote:

> Richard wrote:
> ...
>> I am astonished that people claiming to be professional programmers
>> could be in any way "confused" or "unsure" about your concise
>> replacement for Heathfield's version.
>>
>> ,----
>> | while(*u++=(*s=='.' ? '_' : *s))
>> | s++;
>> `----
>>
>> I simply can not see the complication or the need to "analyse" this.
>> Yes if we were training in lesson 2 we might expand it out a little.
>> but only to the extent of removing the ?: usage to an if then else.

>
> I have interviewed several dozen people for C programming positions
> over the past 15 years. I've given every single one of them a simple
> program to understand and explain and suggest improvements for. The
> heart of that program was the following loop:
>
> while(*p++ = *q++);
>
> Only about half of them could even tell me correctly what that loop
> does; not a single one has ever correctly explained how it does it.
> The only person I've found who ever passed this test wasn't an
> applicant but a co-worker, and she required a lot of hints before she
> figured it out. The stumbling block for most of them is explaining how
> the loop terminates. I've had to hire people who couldn't answer that
> question, because better candidates weren't available.


This is incredible!

<snip>

> You may have had the privilege of working with more competent C
> programmers than I have; but I would recommend not overestimating the
> competence of the average C programmer. The code you cite has every
> feature that made my test difficult for applicants, and adds to that a
> use of the ?: operator.


I agree here. All other factors being equal clearer code is always
better. The issue though is, what's clear to one is obfuscation to
another.

<snip>

 
Reply With Quote
 
Richard Heathfield
Guest
Posts: n/a
 
      10-09-2007
santosh said:

> kuyper wrote:
>

<snip>
>>
>> [classic K&R strcpy loop snipped]
>>The stumbling block for most of them is explaining how
>> the loop terminates. I've had to hire people who couldn't answer that
>> question, because better candidates weren't available.

>
> This is incredible!


No, he's right. Most C programmers are very, very bad at C. That's why it's
so important to write clear, simple code. Or at least, as clear and simple
as possible. No clearer, no simpler.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
 
Reply With Quote
 
Kenny McCormack
Guest
Posts: n/a
 
      10-09-2007
In article <(E-Mail Removed)>,
Richard Heathfield <(E-Mail Removed)> wrote:
>santosh said:
>
>> kuyper wrote:
>>

><snip>
>>>
>>> [classic K&R strcpy loop snipped]
>>>The stumbling block for most of them is explaining how
>>> the loop terminates. I've had to hire people who couldn't answer that
>>> question, because better candidates weren't available.

>>
>> This is incredible!

>
>No, he's right. Most C programmers are very, very bad at C. That's why it's
>so important to write clear, simple code. Or at least, as clear and simple
>as possible. No clearer, no simpler.


I think Sturgeon's law applies here (as it does everywhere).

 
Reply With Quote
 
Malcolm McLean
Guest
Posts: n/a
 
      10-09-2007

"kuyper" <(E-Mail Removed)> wrote in message
> The heart of that program was the following loop:
>
> while(*p++ = *q++);
>
> Only about half of them could even tell me correctly what that loop
> does; not a single one has ever correctly explained how it does it.
>

It's what I call compileable gibberish. That's the sort of thing that gives
C a bad name.
If C were the only lanauge in existence there would be a case for developing
that sort of terse convention. But the fact is that there are many languages
that can copy zero-terminated arrays one to the other, but most require a
slightly more intuitive syntax to do it. Whilst you've got to assume that
the reader knows C, don't assume that he uses the language more than
occasionally. mke is as easy as possible for the reader, and remember that
that trivial inconveniences are cumulative.

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm


 
Reply With Quote
 
Flash Gordon
Guest
Posts: n/a
 
      10-09-2007
Malcolm McLean wrote, On 09/10/07 21:40:
>
> "kuyper" <(E-Mail Removed)> wrote in message
>> The heart of that program was the following loop:
>>
>> while(*p++ = *q++);
>>
>> Only about half of them could even tell me correctly what that loop
>> does; not a single one has ever correctly explained how it does it.
>>

> It's what I call compileable gibberish. That's the sort of thing that
> gives C a bad name.


If you think that then C is not the language for you since it is
idiomatic. Personally I would write it as
while (*p++ = *q++) continue;
I would even suggest the changes at a code review (more spacing would be
mandated by any coding standard I had a say in). Other than that it is fine.

> If C were the only lanauge in existence there would be a case for
> developing that sort of terse convention.


It was developed when C was young (before it was standardised) by the
creators. There were already other languages at the time.

> But the fact is that there are
> many languages that can copy zero-terminated arrays one to the other,
> but most require a slightly more intuitive syntax to do it.


So has C, you will find details in the documentation for the standard
library. That does not mean the code is not idiomatic.

> Whilst
> you've got to assume that the reader knows C, don't assume that he uses
> the language more than occasionally.


Completely wrong. Kuyper was talking about interviewing people for C
programming jobs. Anyone in such a job will be using C a *lot*. Either
that or they will be sacked for not doing the job they are paid for.

> mke is as easy as possible for the
> reader, and remember that that trivial inconveniences are cumulative.


You makes it as easy as possible for the correct level of reader. Anyone
with a C programming job should be able to work out what that loop does
during an interview.
--
Flash Gordon
 
Reply With Quote
 
Antoninus Twink
Guest
Posts: n/a
 
      10-09-2007
On 9 Oct 2007 at 20:40, Malcolm McLean wrote:
>
> "kuyper" <(E-Mail Removed)> wrote in message
>> The heart of that program was the following loop:
>>
>> while(*p++ = *q++);
>>
>> Only about half of them could even tell me correctly what that loop
>> does; not a single one has ever correctly explained how it does it.
>>

> It's what I call compileable gibberish. That's the sort of thing that gives
> C a bad name.


It's the sort of thing that attracted me to C in the first place. I
suspect I'm not alone.

> If C were the only lanauge in existence there would be a case for developing
> that sort of terse convention. But the fact is that there are many languages
> that can copy zero-terminated arrays one to the other, but most require a
> slightly more intuitive syntax to do it. Whilst you've got to assume that
> the reader knows C, don't assume that he uses the language more than
> occasionally. mke is as easy as possible for the reader, and remember that
> that trivial inconveniences are cumulative.


If a maintainance programmer doesn't have a basic level of competence in
the language of the code he's working with, then slight befuddlement
over how string copying works is likely to be the least of his problems
(and the least of the damage to the code that results). I mean,
honestly, this sort of idiom is like Chapter 2 of K&R or something.

 
Reply With Quote
 
kuyper
Guest
Posts: n/a
 
      10-09-2007
Malcolm McLean wrote:
> "kuyper" <(E-Mail Removed)> wrote in message
> > The heart of that program was the following loop:
> >
> > while(*p++ = *q++);
> >
> > Only about half of them could even tell me correctly what that loop
> > does; not a single one has ever correctly explained how it does it.
> >

> It's what I call compileable gibberish. That's the sort of thing that gives
> C a bad name.


I agree that it's an obscure idiom, but I wouldn't go so far as to
call it gibberish.

When I first used this test on an applicant, I expected that anyone
adequately qualified for an entry-level full-time C programming
position would be able to figure out how that idiom works, if given a
reasonable amount of time to do so. Furthermore, since the idiom is
explained in full detail in K&R, I expected that many of them would
actually recognize it. My expectations have lowered since then. I
still think that entry-level programmers should be able to figure this
out, even though I no longer expect them to be able to do so.

....
> slightly more intuitive syntax to do it. Whilst you've got to assume that
> the reader knows C, don't assume that he uses the language more than
> occasionally.


The code I'm responsible for is intended to be read and modified only
by full-time professional C programmers. While the code is publicly
available, making it easily modifiable by people who aren't very
familiar with C is not a priority. It's not just a low priority; it's
completely missing from our list of priorities.

Making it easily modifiable by people who are very familiar with C is
one of our priorities. The depressing thing is how low a limit that
places on the complexity of the code.

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Designing fgetline - a perspective Richard Harter C Programming 48 11-25-2007 11:40 PM
Re: Bug/Gross InEfficiency in HeathField's fgetline program Dik T. Winter C Programming 2 11-07-2007 01:38 PM
Program inefficiency? hall.jeff@gmail.com Python 17 10-01-2007 04:48 PM
RE: Program inefficiency? Michael.Coll-Barth@VerizonWireless.com Python 6 10-01-2007 06:22 AM
Segfaulting when trying to create custom 'fgetline' function Jeff Rodriguez C Programming 4 11-17-2003 08:31 AM



Advertisments