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

Sergey Organov [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Sergey Organov [off-list ref] writes:

quoted

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 ;-)