David Rientjes [off-list ref] writes:
When I read "x > 0", my mind parses that very easily. When I read "0 <
x", it takes me a few cycles longer. I think the goal of any software
project is to not only emit efficient and quality code, but also code that
can be read and deciphered with ease unless it's impossible otherwise.
Well, the thing is, I end up being the guy who needs to stare at
git code longer than you do ;-).
Before --stat-width was introduced there was code like this:
if (max + len > 70)
max = 70 - len;
Here "len" is the width of the filename part, and "max" is the
number of changes we need to express. The code is saying "if we
use one column for each changed line, does graph and name exceed
70 columns -- if so use the remainder of the line after we write
name for the graph". Your "constant at right" rule makes this
kosher.
If we make that to a variable, say line_width, we can still
write:
if (max + len > line_width)
...
I however tend to think "if line_width cannot fit (max + len)
then we do this", which would be more naturally expressed with:
if (line_width < max + len)
...
Now, at this point, it is really the matter of taste and there
is no real reason to prefer one over the other. Textual
ordering lets my eyes coast while reading the code without
taxing the brain. I can see that the expression compares two
quantities, "line_width" and "max + len", and the boolean holds
true if line_width _comes_ _before_ "max + len" on the number
line (having number line in your head helps visualizing what is
compared with what). If you write the comparison the wrong way,
it forces me to stop and think -- because on my number line
smaller numbers appear left, and cannot help me reading the
comparison written in "a > b" order.
I could try writing constants on the right hand side when
constants are involved, but I do not think it makes much sense.
It means that I would end up doing:
- if (max + len > 70)
- max = 70 - len;
+ if (line_width < max + len)
+ max = line_width - len;
Consistency counts not only while reading the finished code, but
also it helps reviewing the diff between the earlier version
that used constant (hence forced to have it on the right hand
side by your rule) and the version that made it into a variable.
To change the code itself because of a hard 80-column limit or because
you're tired of hitting the tab key is poor style.
Well, the program _firstly_ matches the logic flow better, and
_in_ _addition_ if you write it another way it becomes
unnecessarily too deeply indented. So while I agree with you as a
general principle that indentation depth should not dictate how
we code it does not apply to this particular example.
On Tue, 26 Sep 2006, Junio C Hamano wrote:
Well, the thing is, I end up being the guy who needs to stare at
git code longer than you do ;-).
Really? This is the only community that hacks git? There _are_ people
out there that make their own changes specifically tailored to their
purposes or that of their organization.
Before --stat-width was introduced there was code like this:
if (max + len > 70)
max = 70 - len;
Here "len" is the width of the filename part, and "max" is the
number of changes we need to express. The code is saying "if we
use one column for each changed line, does graph and name exceed
70 columns -- if so use the remainder of the line after we write
name for the graph". Your "constant at right" rule makes this
kosher.
If we make that to a variable, say line_width, we can still
write:
if (max + len > line_width)
...
I however tend to think "if line_width cannot fit (max + len)
then we do this", which would be more naturally expressed with:
if (line_width < max + len)
...
First of all, it's not my "constant at right" rule, it's a preference that
the _majority_ of computer programmers have used in virtually every
language that you see source code for.
The grammar for C is
relational-expression:
shift-expression
relational-expression < shift-expression
in this case. Now while this supports both your variations above, it
_suggests_ that the higher degree of computation is associated on the left
side because the less-than operator associates that way.
What happens here:
a < b < c
it turns out that this is equivalent to:
(a < b) < c
so if you want your entire code base to conform to a particular style,
it's _preferable_ to place the constant on the right. And that's what the
majority of programmers do. Your taste is in the minority and out of
respect to the code base you should make your code conform to what is most
popular in the surrounding code.
Your argument of saying to yourself "if line_width cannot fit max + len
then we do this" has no relevance at all. I can say "if max + len is too
big for line_width we do this" just the same.
If we're going by what sounds better in your head, then I expect _no_
argument when I write a function called conseguir_la_linea_longitud
instead of get_line_length because Spanish is my first language.
Please respect what the majority of computer programmers write and unify
the code base so that it's a similar style everywhere.
quoted
To change the code itself because of a hard 80-column limit or because
you're tired of hitting the tab key is poor style.
Well, the program _firstly_ matches the logic flow better, and
_in_ _addition_ if you write it another way it becomes
unnecessarily too deeply indented. So while I agree with you as a
general principle that indentation depth should not dictate how
we code it does not apply to this particular example.
This is a ridiculous argument. The C code will emit the exact same
assembly regardless of how you write it. You say that you wrote it that
way to avoid idents which is an absolutely horrible way to dictate the
code you use. There are tons of opportunities where you can write cryptic
source code that functions great with the least number of tokens and least
number of lines to get the job done in every large project. But, given
that there are no assembler or performance tradeoffs, it should be written
as clearly and nicely as possible for the reader. I assert again what I
did previously: if that if clause runs the length of my screen the indents
will help me later to remember we're still in a conditional. That's the
SOLE purpose of indents: to make it easy for the reader to tell you're
inside a block.
And in one of your patches you had:
if (...)
;
else {
...
}
without any other if statements. If you're supporting that type of code,
I'll simply consider this entire thread a lost cause.
David