4 ms·

On second thought, let's not go to Camelot. It is a silly place. It's useful shorthand in a few specific cases, but I would argue that this sort of code is ba

• caf 15y ago
  The problem is that if you try to rewrite that example to avoid the comma operator, you either have to duplicate the update_thing(&foo) call, as in: update_thing(&foo); while (foo != 0) { /* loop body */ update_thing(&foo); } Unnecessary duplication like this is itself a potential source of bugs - it's all to easy to update one but not the other. The other alternative is to hack up the loop to exit in a strange place: while (1) { update_thing(&foo); if (foo == 0) break; /* loop body */ } This is arguably even worse - the actual loop termination condition is not where you expect to find it anymore. Personally, I find the formulation using the comma operator to be completely clear. Note that the bug referenced here is more about the subtleties of bitfield type promotion in expressions, and the interplay of bitfields with operators that evaluate to the type of one of their arguments, than it is about the comma operator. You can show the same bug using the assignment operator instead of the comma operator. If you're going consign anything involved here to the "never do this" section, I'd start with any use of bitfields, and maybe also include mixing unsigned and signed types in expressions without explicit conversion.
  • tb 15y ago
    I completely agree with everything you've said except for "any use of bitfields." The comma operator is ok in this circumstance, mixing unsigned and signed is a big no-no, but what have you got against bitfields? They're very useful when it comes to pulling sub-byte fields out of network packets.
    • nitrogen 15y ago
      They're very useful when it comes to pulling sub-byte fields out of network packets. That is, if you don't mind tying your code to a particular platform and compiler. The alignment and packing of bitfields is implementation dependent: http://stackoverflow.com/questions/1490092/c-c-force-bit-field-order-and-alignment http://stackoverflow.com/questions/1490092/c-c-force-bit-fie... For what it's worth, I use bitfields on occasion, but only to save memory and get better warnings when storing range-limited values. It's nice to have the compiler warn about a comparison always being true or false due to the limited range of a type. The performance of bitfields is probably worse than just tossing all my boolean values into an int32_t or int8_t.
      • tb 15y ago
        Aha, thank you. As most of my C code has been written for a single compiler and a single platform I was less aware of the pitfalls than I should have been. I will know now to be extra vigilant if I ever have to port that code.
    • caf 15y ago
      They're not any use for pulling sub-byte fields out of network packets in portable code, because everything about the precise layout of bitfields is entirely implementation-defined. Portable code has to use masking-and-shifting. The only portable uses of bitfields are to potentially save a little memory, and to get "modulo-power-of-2" behaviour. Bitfields are a classic "not as useful as they first appear" feature.
  • acqq 15y ago
    We're coming in the dangerous area of personal preferences, but with the decades of writing and also maintaining code, I certainly prefer "exit in a strange place" variant, as it clearly better allows the code to "grow" with less differences between revisions, like adding a few more lines between update_thing and if. And if the revisions are not expected then how is written is in my opinion realy irrelevant. (Disclaimer: I won't discuss this subject more here.)
  • zokier 15y ago
    do { update_thing(&foo); } while (foo != 0);
    • jws 15y ago
      This works for the example as typed, but not if there is a body to the original loop.
      • onemoreact 15y ago
        A little ugly but you can get it to work without duplicate code by: bool second_loop = false; do { if (second_loop) {...} update_thing(&foo); second_loop = true; } while (foo != 0); The real question is which is more clear when some else looks at the code. I suspect even if it's shorter and faster while (foo,bar) is less clear to most people than the above code.
  • bartwe 15y ago
    Loop-and-a-half is a very common and useful pattern, I don't see anything wrong with the second code snippet.
• 1amzave 15y ago
  Well, the problem here is (assuming I'm understanding everything correctly) pretty specifically the combination of the comma operator and a bitfield. Of those two, I'd say the latter, even if superficially more intuitive, is probably the more obscure and advisable to avoid. The comma operator, while not all that common, can be pretty useful in common situations, and can easily achieve both clarity and conciseness. For example, to iterate over a singly-linked list while keeping a pointer to the previous item: for (prev = NULL, this = first; this != NULL; prev = this, this = this->next) { ... } I think this is a perfectly reasonable way to do things. Disallowing the comma operator due to an underspecified interaction with bitfields would be throwing out the baby with the bathwater, in my opinion.