Re: [PATCH] describe --contains: default to HEAD when no commit-ish is given

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

SZEDER Gábor <szeder@ira.uka.de> writes:

>> By the way, I usually prefer a fatter 'else' clause when everything
>> else is equal, i.e.
>>
>> 	if (!argc)
>>         	argv_array_push(&args, "HEAD"); /* default to HEAD */
>> 	else {
>> 		while (*argv) {
>>                 	...
>> 		}
>> 	}
>>
>> because it is easy to miss tiny else-clause while reading code, but
>> it is harder to miss tiny then-clause.  In this case, however, the
>> while loop can be replaced with argv_array_pushv() these days, so
>> perhaps
>>
>> 	if (!argc)
>>         	argv_array_push(&args, "HEAD"); /* default to HEAD ... */
>> 	else
>> 		argv_array_pushv(&args, argv); /* or relay what we got */
>>
>> or something?
>
> Indeed, I didn't notice argv_array_pushv() being added, log tells me
> it happened quite recently.  I suppose with both branches becoming a
> one-liner the order of them can remain what it was in the patch,
> this sparing the negation from 'if (!argc)'.

Another reason to favor the way the code is illustrated for
educational purposes above is it is easier to see exceptional case
first, i.e. "if we have nothing then we do this special thing, but
otherwise we do the normal thing".

But that is a much weaker preference than the preference to "fatter
else"; I could go either way.

> v2 comes in a minute.

Thanks.