Should shortcuts, brevity, and cleverness be routinely skipped in favor of clarity?

This question came to mind after seeing this simple piece of code:

if (!x%y)
{
    // do something
}

Maybe it's the influe开发者_运维知识库nce of early C books (K&R?), but isn't the following always preferred, if not as cute?

if (x%y != 0)
{
    // do something
}

This quote answers your question.

"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

Are you sure about that code? !x%y means (!x)%y because !binds tighter than %.

(For that reason alone, I would prefer x % y != 0.)

First of all, props to everyone noticing that (!x%y) is not equivalent to (!(x%y)), but more importantly, neither of them is equivalent to:

if (x % y != 0)

which has a much nicer form:

if (x % y)

Personally I try not to write ==0 when it can be replaced by use of ! without introducing excessive parentheses, and I absolutely never use !=0, but this is a discussion that will start flamewars. :-)

Cleverness should be skipped. It isn't clever... or more importantly maintainable.

Shortcuts and brevity may or may not be acceptable: we all take shortcuts but there are almost "industry standard" shortcuts because we all do it. No clever shortcuts though

Prefer clarity over brevity. The second example x % y != 0 is more clear.

Of course, what constitutes clarity is somewhat subjective, but I prefer to reserve the unary ! operator for boolean variables or functions returning boolean.

The way I usually try to gauge clarity as I write code is by asking myself: would I easily be able to read and understand this line if 1) someone else wrote it or 2) I had to read it again 2 years down the line.

Clarity wins over brevity every day, in my opinion. I sometimes feel that writing code such as:

if(!(x = func(y)) && ++z == x)

...is the equivalent to go-faster stripes on a car. It may feel fast, but it isn't. I also don't think that using incomprehensible variable names (such as in the above example) to save on typing is a good idea.

What is better here:

for(int x(0) x < managers.size(); ++x)
    managers[x]->initialise();

or:

for(int mgr(0); mgr < managers.size(); ++mgr)
    managers[mgr]->initialise();

They both accomplish the same thing and one could argue that there's no point making mgr more clear (there's even other arguments to say mgr should be clearer :) ). But if ever this part of the routine gets more complicated, it might be very important:

for(int mgr(0); mgr < managers.size(); ++mgr)
{
    for(int dependentMgr(0); dependentMgr < managers[mgr].dependents().size(); ++dependentMgr)
    {
        // init these first
    }
}

Obviously, this is not a discussion on whether to use iterators or not, merely whether we should use clear names or not.

When I look at a piece of code I have not seen before, the variable names are everything. I cringe when I see tmp and tempt2 and vec2. They mean nothing.

Seriously people, if you're worrying about the amount you're typing, get an IDE with autocomplete or go raise chickens in Fiji :)

Actually, if you want your code to be self-documenting (Martin Fowler Style), you can use the more verbose:

every_player_gets_equal_points = x%y;
if (!every_player_gets_equal_points) { //Some players get more: now base on completion time

The first code snippet you present, ...

if (!x%y)
{
    // do something
}

... is meaningless and therefore most likely incorrect, since !x%y is equivalent to !x except for the cases of y equals -1, 0, or 1, and in the y equals 0 case it's Undefined Behavior (i.e., %y is generally meaningless).

Others have already remarked that the first snippet is not equivalent to the second more verbose one. But then one could wonder which snippet expresses the intention. My point is that the first snippet is not equivalent to anything that is likely to be correct.

And so this illustrates nicely that clarity is a good idea.

Cheers & hth.,