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

Junio C Hamano [off-list ref] writes:

Sergey Organov [off-list ref] writes:

quoted

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) { ... }".

Well, I even tend to write this switch(optarg) as:

if (0) ;
else if(...) {
  ...
}
else if(...) { 
  ...
}

to make the first actual "case" look the same way as the rest, and then
through suitable defines it finally looks like:

IF_SWITCH
IF_CASE(...) {
  ...
}
IF_CASE(...) {
  ...
}

making the intent explicit.

Just saying.

Thanks,
-- Sergey Organov