18

Sometimes an algorithm can be written in two ways:

  • The short, fancy way; or
  • The longer, easy-to-understand way.

For example, here is a longer, easier way of copying a string source to dest in C:

*dest = *source;
while (*source != '\0') {
    source++;
    dest++;
    *dest = *source;
} (true);

And here is a short, fancy way.

// Copy string source to dest
while (*dest++ = *source++);

I've always heard and read that fancy code should be avoided, and I tend to agree. But what if we take comments into account? Assume that, as in the examples above, we have an uncommented, longer and supposedly easier-to-understand code, and a well-commented, short, fancy code? Is the non-fancy code still preferred?

EDIT: Many have commented on variable names, so I've modified the example code as to not make that a factor when preferring on over the other. I tried to remove the double assignment in the first example, but that only made the code less readable.

Perhaps this wasn't the best of examples because many find the 'fancy' code more readable and understandable than the longer code. The idea was to have one longer code which was much easier to understand than a very short but complicated code.

EDIT2: Here's a new examle I got from SO:

Commented fancy version:

//direct formula for xoring all numbers from 1 to N
int Sum = (N & (N % 2 ? 0 : ~0) | ( ((N & 2)>>1) ^ (N & 1) ) );

Non-commented long version:

int Sum = 0;
for (int i = 1; i < N; ++i)
{
   Sum ^= i; //or Sum = Sum ^ i;
}
7
  • 2
    @gablin: "goodly" is a word, but it's an adjective with a somewhat different meaning from "good," and isn't widely used anymore (at least in the U.S.) The adverb form of "good" is "well," as in "well-commented code." Oct 4, 2010 at 15:41
  • @John: Ah yes, of course. I've updated the question accordingly. Thanks.
    – gablin
    Oct 4, 2010 at 17:08
  • 2
    In what way is the short way 'fancy'? That is a text-book piece of C code that anyone who claims to know C should be able to read without any issue. Terseness does not mean 'fancy'. Oct 15, 2010 at 20:43
  • @JBRWilkinson: As I said in the modified part of the question, this example isn't a good one since the "fancy" version apparently isn't all that fancy. The aim was to have one short but not-understandable way with comment and then a longer but much more readable way with no comment.
    – gablin
    Oct 16, 2010 at 7:44
  • @gablin: Your edit compromises the current answers.
    – Maniero
    Oct 18, 2010 at 21:57

12 Answers 12

28

I would generally prefer to extract the fancy code out into its own method..

Rather than comment the fancy code, its method name should be all it needs to make things clear.

char *copy_string(char *s, const char *t) {    
    while (*s++ = *t++); 
    return s;
}
7
  • 3
    Great point. Factoring out self-contained code is far better than comments. Compiler knows how to inline it if needed.
    – dbkk
    Oct 4, 2010 at 11:21
  • Ah yeah, that's a great way of making the comment obsolete. But despite this, would you always prefer the fancy code over the longer code?
    – gablin
    Oct 4, 2010 at 15:03
  • In general, yes. I feel that short, well named methods make code easier to read and maintain. Complexity factored out into its own methods makes reuse more accessible and apparent. There are always exceptions tho, ultimately it always depends on the code. Oct 4, 2010 at 15:58
  • Surely your function should be named 'strcpy' ? Oct 15, 2010 at 20:39
  • There's an error in that piece of code. What is returned? What was supposed to be returned? Just make it a void method and be done with it ;) Oct 16, 2010 at 4:33
10

I'm all for the longer version. The problem with the short version of the code, aside from it being more difficult for some programmers to read, is that most of the time it is harder to spot mistakes by just looking at it.

I have a real life example. In our product we had the following snippet of code:

if (++someCounter < MAX_VALUE) {
    // Do something that has to be done only MAX_VALUE times
}

This code looks perfectly reasonable at first sight, but after a long period of time this counter overflows and breaks the condition (in the real-life scenario we crushed our client's production system with an OOM). This code is a bit less "sophisticated", but it's clear that it does what it is supposed to do:

if (someCounter < MAX_VALUE) {
    ++someCounter;
    // Do whatever it is we came here for
}

And you may say that the developer who wrote this code just wasn't good enough (which is not true, he was a rather bright guy), but I think that if your coding technique requires you to be super-duper skilled programming GOD in order to get your code right, you are doing something wrong. Your code should be as fool-proof as possible for your sake and for whoever has to maintain this code after you (who may not be as smart as you are).

So - I'm all for simplicity and clearness.

Edit: Oh, and regarding comments - it doesn't matter. Many people won't read comments anyway (http://www.javaspecialists.co.za/archive/Issue039.html) and those who won't understand your code without comments, will not understand it well enough with them so that they can maintain it. The goal is to help people see that certain code is "correct", comments can't help with that.

4
  • 1
    "crushed our client's production system" - that is pretty bad :-S
    – user1249
    Oct 17, 2010 at 21:07
  • "The problem with the short version of the code, aside from it being more difficult for some programmers to read" -- any piece of code can be hard "for some programmers" to read. You just need to find stupid enough programmers. Don't dumb down the code to the lowest denominator.
    – quant_dev
    May 16, 2012 at 13:08
  • @quant_dev: conversely, "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan May 16, 2012 at 15:35
  • @MichaelBorgwardt I wouldn't apply Kernighan's dictum blindly to every possible situation. The OPs example is a function written "as cleverly as possible" (or close to it) and yet it should be pretty easy to debug, if any debugging is needed. On the other hand, complicated bit-twiddling oneliners can be very clever, but will certainly be even harder to debug. (Also: Kernighan assumes that coding skills = debugging skills. It doesn't have to be the case. I've successfully debugged code I would not have been able to write.)
    – quant_dev
    May 16, 2012 at 15:49
6

I would generally prefer the longer version. Two main reasons spring to mind:

  • More people are likely to understand it with less effort (assuming it's not such a "standard" example as this string copy).
  • It's possible to put breakpoints on individual statements and step through with a debugger.

For the best of both worlds, wrap the code in a function, whose name does the commenting for you:

void copy_string(char *s, char *t)
{
    *s = *t;
    while (*t != '\0') {
        t++;
        s++;
        *s = *t;
    }
}

The only reason not to do this would be if performance were an issue and profiling genuinely showed the function overhead to be significant.

2
  • 1
    Isn't that what inline is for?
    – Antsan
    Oct 4, 2010 at 11:05
  • Indeed, although inlining comes with its own share of problems, like having to expose the function body in header files. Don't modern compilers inline stuff for you automatically where sensible anyway? Oct 4, 2010 at 11:14
4

Whatever is clearest. Often that's a compact easy to understand snippet of code that doesn't require a comment... like your second example.

Generally shorter snippets of code are easier to understand as there is less for the reader to keep in his head at a time. Obviously, there is a limit when the code gets overly obfuscated and I don't mean that clarity enhancing white space should be trimmed.

Comments should never state anything that is obvious from the code, that just makes the reader read the same thing twice. To me the first snippet requires clarification. Why hasn't the developer used a do...while loop to remove the duplicated of the *s = *t; assignment? I have to parse more code to realise that it's implementing a string copy. A comment would be more helpful on this longer code than on the shorter code.

The comment on the second snippet is almost redundant as that while loop is practically idiomatic, but it does say what the code does at a higher level that the code itself which makes it a useful comment.

1
  • The developer (me) hasn't used do ... while simply because I didn't think of it when I wrote the question. Thanks for pointing that out. ^^ The second snippet may be obvious to you, but it certainly is not for me.
    – gablin
    Oct 4, 2010 at 14:52
4

The problem is that there is no clear definition of short fancy code since it depends very much on the level of the programmer. While some people have no problem understanding a while(*s++ = *t++); expression, others will.

Personally I find the while(*s++ = *t++); perfectly readable and the longer one more difficult to read. Others might not agree though.

Regardless, it's just a matter of using common sense. Readability should definately be a priority but there's a point where longer code gets less readable when it gets longer. Verbosity often reduces readability from my experience.

3
  • It makes me sad that a theme of many of these answers is " developers may not recognize the standard way of writing strcpy() in C"
    – AShelly
    Oct 4, 2010 at 15:41
  • It shouldn't - it doesn't mean that there are no more good programmers, rather that a) we don't teach C much any more and b) that the strength of C is also its weakness - in that whilst that loop is understandable to one practiced in C its also pretty arcane and in a world endeavouring to write secure code fairly scary for what it implies about what you can do in C
    – Murph
    Oct 4, 2010 at 16:47
  • So we find ourselves with another challenge to our daily tasks. Needing to guess the general level of language comprehension of our direct peers so that our code reviews will remain constructive. Not falling into battles of which is the "correct", cleaner expression of intent. Nov 12, 2010 at 6:58
2

I have to disagree with most of the other answers here -- I think (at least in this case) the shorter code is better. Contrary to your claim, the longer code is not "easier", at least for a reader. If anything, you seem to have gone out of your want to make it longer, even though it makes the code more difficult to understand and/or be assured of correct operation.

In particular, having the assignment of the first byte of the string outside the loop, separate from the assignment for the other bytes, means one has to be much more careful in reading to be sure that all the bytes are copied correctly. The basic sequence of actions in the shorter version is much easier to verify. Its only real issue is in the formatting -- when you have an intentionally-empty loop body, it's best to make that clear, something like:

while (*dest++ = *source++)
    ;

or even:

while (*dest++ = *source++)
    ; /* no body */

Some people prefer to use braces instead:

while (*dest++ = *source++)
    {}

Regardless of the exact formatting you prefer, enough mistakes have been made with things like:

if (x>y);
     x = z;

...that it's important to be sure that 1) it's obvious what's actually controlled by any flow-control, and 2) it's obvious that the code was written knowing what was controlled by it, so somebody reading it doesn't waste time trying to figure out whether they've just found a bug.

1
  • Yes, in retrospect of this particular example, the 'fancy' version is probably more readable than the longer version. If I could think of a better example I would, but I am unable to at the moment. But I didn't "go out of my way" of making it longer - that is how I would write a string copy, at least initially. It may not be the best approach, but it wasn't made longer than necessary by intention.
    – gablin
    Oct 4, 2010 at 17:11
2
Keep it simple, stupid!

Make sure any other programmer can understand what your code does—and even better if it's at a glance (this is where good names and comments come into their own).

Fancy bit-twiddling kung-fu and inlined assembly is all great in the name of (perhaps unnecessary and premature) optimisation, but when even you can't understand the code you wrote when you come across a bug in it two months later... What was the point? You'll cost yourself time and effort.

I often refer to the Zen of Python in these situations. In particular:

Explicit is better than implicit.
Simple is better than complex.
1

Don't write fancy code in production software. I know it feels good to be able to actually write that, and it's shorter. Writing fancy code significantly increases the "WTF/minute" value, in other words decreases quality.

alt text

1

It of course entirely depends upon the circumstances. but in general, i find this to be a good rule of thumb:

Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.

~ Brian Kernighan

1
  • longer but more meaningful, elegant even, than the more compact KISS acronym ~ hope the irony there is not lost ;p
    – violet313
    May 17, 2012 at 2:33
0

In my experience the biggest win in terms of short fancy code tends to be using recursion rather than iteration. It's also one of the hardest thing to understand at a glance-over of code, unless you're working in the kind of language where everything is recursive anyway. I would still favour it for the elegance and speed of development it offers, but I try to make sure it has detailed comments if it looks like it will be opaque to future maintainers or my future self.

0

I'm starting to never trust comments. All too often, comments don't get updated when code gets updated and are vastly out of date or represent rules from clients/management that no-longer are relevant.

It has gotten to the point in some cases where the comments don't even match the code that they're describing anymore.

If I have to choose between short/fancy and longer/easier to understand then I always choose the latter unless there is a really good reason.

0

Readability and maintainability are key, and your second example (i.e. the longer one) is much more of both. But why limit yourself to two options? Even the longer code is too complex (IMHO) to not notate further. I'd put it in a method of its own with appropriate Javadoc (or whatever) and a suitable method name.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.