don fong wrote:
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);
     report_error (_("%s: parameter null or not set"), name);
1) I see no benefit in the use of extra braces.  It diminishes
comprehension in this case.

2) The purpose appears to, optionally treat a null value for ptr name
as either an "unset" condition (as in never set) vs. mentioning
that it might also be the case that it might have been set, but with
a null value.

OTOH, if the purpose is to vary the error message, I might find
1 call to be more clear:

   report_error( check_null ? _("%s:  parameter null or not set")
                            : _("%s:  parameter not set"),    name );

Whether or not 'name )' would be on a separate line, or set off
with extra spaces would depend on code width compared with surrounding
lines.  That also assumes whether or not the macro '_()' can be
used more than once within the call to report_error.  It might be
that the format above would exceed some desired code width, which
might "recommend" a different formatting.

It's hard to say w/o knowing other conventions in the code.

However, personally, I'd find the fact that it was accepted and
simply adapted to whatever the code owner wanted in this situation, an
overriding benefit.  Owners that require submitters to
always use the owner's exact formatting (including tab/space
and indentation preferences) will tend to dissuade contributions.

I have things that are more clear for me to read and comprehend,
while others have their own "input format" that benefit them as well.
Ideally, computers can be used to automatically reformat such
differences as code is imported or exported.

Reply via email to