admittedly this is a very minor point, but i am curious. this has to do
with coding standards for bash source.
consider an if statement in C (or bash, for that matter). which is form is
better?
Form (A):
if (flag)
X();
else
Y();
Form (B):
if (flag == 0)
Y();
else
X();
they are functionally equivalent. but IMHO (A) is slightly more readable.
first because flag (in this case) is intended to be a boolean value not
arithmetic, and second because it's simpler to think about an if when the
condition is positive.
this is what i'd say if (B) were under code review.
i submitted a patch with code in form (A). it was added to the code base
in form (B). was there a good reason for this mutation?
NOTE: i'm OK with the fact that alterations were made. but i wonder what
was the reasoning? as near as i can tell, my patch could have been just
applied verbatim, but it wasn't. and i don't see how the alterations were
an improvement.
all of which makes me wonder what is the review process for changes to the
bash source?
here is the actual code from which the above example was derived, in
subst.c.
my patch (form (A)):
- report_error (_("%s: parameter null or not set"), name);
+ {
+ if (check_nullness)
+ report_error (_("%s: parameter null or not set"), name);
+ else
+ report_error (_("%s: parameter is not set"), name);
+ }
the new code (form (B)):
else if (check_null == 0)
report_error (_("%s: parameter not set"), name);
else
report_error (_("%s: parameter null or not set"), name);
a couple of other minor alterations were made.
* my code used the same variable name check_nullness which was used in the
calling routine, vs check_null in the altered version. i'm OK with it even
though i think using the same variable name to stand for the same thing
would be slightly better.
* my code's error message was a sentence, "parameter *is* not set" vs the
terse "parameter not set". there is a certain consistency to omitting the
verb, if you don't care about clarity. i'm OK with this.
