Re: [PATCH 1/3] diff-merges: cleanup func_by_opt()

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

Sergey Organov <sorganov@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>> Get rid of unneeded "else" statements in func_by_opt().
>>
>> While it is true that loss of "else" will not change what the code
>> means, this change feels subjective and I'd say it falls into "once
>> committed, it is not worth the patch noise to go in and change"
>> category, not a "clean-up we should do".
>
> I agree the "else" vs "no else" is subjective, but the problem in fact
> is that the first "if", unlike the rest of them, already had no "else",
> making the code inconsistent.

This is a static helper function about a single "optarg" string that
wanted to say "switch(optarg) { case "off": ... }" but couldn't in
C, and I happen to view if strcmp else if strcmp ... sequence on the
same string a poor-man's substitute for such a construct.  So my
take of it is to call the second "if" not being "else if" a problem,
not the rest of it.  If we add a new condition on a different input,
making it "if (x) ...; switch(optarg) { ... }" that talks about
something other than optarg, then writing it all with "if" without
"else if" would make it harder to see the pattern, but I do not care
too deeply either way, because this is unlikely to gain any logic
more involved than "switch(optarg) { ... }".

> So the fix should either be adding one
> "else" to the first "if", or remove all of the "else". I chose the
> latter, to end up with less noisy code.

Yup, see above for the reason why I would choose else-if cascade if
I had to but I do not care too deeply either way in this particular
case ;-)