Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Reading Code Complete (and listening to Crockford's talks) has really opened my mind to writing clearer code constructs. For example, the for-loop's job in the first example should be to track indexes. There shouldn't be code that "does stuff" between parens. Instead of superficially breaking the for-loop into several lines and wasting time on aligning semicolons, it could be re-written as a while loop with clarity in mind. Like this:

  while (*from != 0) {
    *to = *from;
    from++
    to++;
  }
To me this looks much saner (unless I'm doing something wrong, I'm a bit rusty on pointers). But I notice that a lot of "C-hackers" try to cram as much into one line as they can, often including every possible pointer incrementation and assignment. At least here, the variables are properly named. A lot of C code uses one-letter variables and reading those isn't a lot of fun, e.g.

  while((*t++ = *f++) != 0 ) 
(Note, I don't really know if this is correct.)


Yes, a for loop is ridiculous for a string copy; K&R do it like so:

  while (*t++ = *f++)
      ;
The pointless comparison to 0 is removed, and the semi-colon is required to terminate the statement. Of course, the point about this version is, once you understand it, it's trivial to recognise. Parsing the 'full' version, especially as formatted in the article, takes a lot longer because it's not familiar and contains more parts to read, any of which could deviate from what might be expected. The other positive is much more code is visible at once.


I guess your example is so idiomatic that it's easy to recognise what it does at a glance. It's good to use easily recognisable patterns. The problem is that I really feel that pointer arithmetic should be separate, because in more complicated code the above style might lead to off-by-one errors that are hard to detect. Also, I don't think the comparison with 0 is pointless. I feel that you should only remove the comparison when an expression gives a boolean result. One other problem here is that, at least for me, it seems safer to only write termination conditions inside parens and the copying code in the loop's body.


Maybe the reason "C-hackers" do it that way is because it actually works, while your example completely fails to null-terminate the destination string.


Good point. I guess it gets a bit more complicated when you try to do the right thing. So I'll re-examine the simple while loop like oneeyedpigeon posted:

  while (*t++ = *f++)
      ;
This code really packs a lot of punch. The value of f is copied to t and then both pointers are incremented. If the value is 0 the loop is terminated. So there's always at least one character copied.

In my initial rash response the loop would exit without copying the 0. So to fix it I might just add a new line

  *to = 0;
(if I'm not mistaken the pointer is already incremented when the loop exits).

Another option would be a while loop with a break statement, It looks weird, but does express the correct intent, which is "continuously copy from source string and exit if you've reached the end":

  while (true) {

    *to = *from;
    if (*to == 0) break;

    from++
    to++;

  }




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: