Re: [PATCH v2] cache_tree_find(): remove redundant checks

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

>> Isn't the above a strchrnul()?
>
> Oh, cool, I never realized that this GNU extension was blessed for use
> in Git.  Will change.

We do have our own fallbacks for non-glibc platforms, so it should
be safe.

>> Combining a freestanding decl with intializer assignment to lose one
>> line is sort of cheating on the line count, but replacing the three
>> lines with a single strchrnul() would be a real code reduction ;-)
>
> I suppose you are just teasing me, but for the record I consider line
> count only a secondary metric.  The reason for combining initialization
> with declaration is to reduce by one the number of times that the reader
> has to think about that variable when analyzing the code.
> ...
> I really wish we could mix declarations with statements because I think
> it is a big help to readability.

Unfortunately, I think we are in violent disagreement.

A variable declaration block with initializations on only some but
not all variables is extremely annoying.  If none of the variable
declaration has initialization (or initialization to trivial values
that do not depend on the logic flow), and the first statement is
separated from the decl block, then I do not have to read the decl
part when reading the code/logic *at all* (the compiler will find
missing variables, variables declared as a wrong type, etc.).

In other words, a trivial initialization at the beginning of the
block, if the logic flow only sometimes makes assignment to the
variable, is perfectly fine, e.g.

	const char *string = NULL;

	if (...) {
        	string = ...
	}                

But I would wish people stop doing this:

	const char *string = strchrnul(name, ':');

	... the real logic of the block that uses string follows ...

and instead say

	const char *string;

	string = strchrnul(name, ':');
	... the real logic of the block that uses string follows ...