Ben Collins-Sussman wrote:
> + if (apr_err)
> + {
> + return svn_error_wrap_apr
> + (apr_err, _("Can't change working directory to '%s'"), base_dir);
> + }
>
> No need for curly braces around a single-line block.
>
[...]
> if (sys_err != 0)
> - /* Extracting any meaning from sys_err is platform specific, so just
> - use the raw value. */
> - return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
> - _("system('%s') returned %d"), cmd, sys_err);
> + {
> + /* Extracting any meaning from sys_err is platform specific, so just
> + use the raw value. */
> + err = svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
> + _("system('%s') returned %d"), cmd,
> sys_err);
> + }
>
>
> Same thing here.
Actually, Ben, this has always been something we've left up to individual
preferences -- we have no stylistic convention on this matter, and never
have. For one thing, given a block of code like:
if (some_cond)
/* This is a comment */
return (some_stuff);
some would think of that as a one-liner inside the if(), others think of
"lines" like physical lines of text. Arguably, it'd be easier to read this
"one-liner" if it was wrapped in braces.
if (some_cond)
/* It is for us the living rather to be dedicated here to the
unfinished work which they who fought here have thus far so nobly
advanced. It is rather for us to be here dedicated to the great
task remaining before us--that from these honored dead we take
increased devotion to that cause for which they gave the last full
measure of devotion--that we here highly resolve that these dead
shall not have died in vain, that this nation under God shall have
a new birth of freedom, and that government of the people, by the
people, for the people shall not perish from the earth. */
return (some_thing);
Other thoughts (YMMV): including the braces up front means that if the body
of the condition grows to more than one line, the diff need only contain the
new logic, not the new formatting of the old logic, too (because indentation
levels change). And there are other consideration, too, like how to format
else blocks -- do you pair an unbraced if() with a braced else, or keep them
visually balanced. Stuff like that. (See even the example in hacking.html
immediately inside the "Coding style" section, as well as inconsistent
examples inside "Exception handling".)
All this to say, try to avoid passing off your personal preference as
Subversion-wide convention. Though, in this case, I suspect you just really
thought such a convention existed.
--
C. Michael Pilato <cmpilato@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Mon Jul 23 15:51:24 2007