[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] A matter of style
From: |
Greg Chicares |
Subject: |
Re: [lmi] A matter of style |
Date: |
Fri, 23 Feb 2007 12:36:50 +0000 |
User-agent: |
Thunderbird 1.5.0.4 (Windows/20060516) |
On 2007-2-22 20:15 UTC, Ericksberg, Richard wrote:
> On 2007-02-22 13:00 UTC, Greg Chicares wrote:
>>
[Instead of:]
>> if(IsVoid())
>> {
>> return;
>> }
>>
>> [...] I'm starting to think that
>>
>> if(IsVoid())
>> {return;}
>>
>> might be better. Reason: by saving two lines, that alternative
>> may make function bodies shorter and therefore easier to read,
>> particularly if there are multiple early exits.
[...]
>
> I like this idea but would like to clarify when we should use
> one format vs. the other? Right now I'm assuming this one would
> be used only when it's 'likely' to just be a single line
> [e.g. return, fatal_error() etc.]
Another factor to consider is whether it's likely ever to change.
That factor is often correlated with brevity, as in your examples:
> if(whatever)
> {return;}
>
> and this one otherwise
>
> if(whatever)
> {
> DoSomething();
> MaybeDoSomeOtherStuff();
> return;
> }
Yes.
> and not
>
> if(whatever)
> {DoSomething(); MaybeDoSomeOtherStuff(); return;}
Right. That could easily lead to atrocities like
PerformTask0(); PerformTask1(); PerformTask2();
PerformTask3();
I'd generally avoid that, but the example you cite below is an
exception. When the clearest, most-obviously-correct layout
goes against some general guideline, then we don't follow the
guideline robotically. For example, Duff's device may go against
general guidelines, but I use it in 'ncnnnpnn.hpp'; any computer
scientist would recognize it as an idiom.
> Also, what about this format found in main_wx.cpp?
>
> switch(*i)
> {
> case '\n': {*j++ = ';';} break;
> case '\r': { } break;
> case '\t': {*j++ = ' ';} break;
> default : {*j++ = *i;}
> }
"Replace newline with semicolon;
drop carriage return; and
replace tab with space; but
ignore anything else."
Write it any other way, and I think it'd be harder to understand.
This is formatted in two dimensions, with attention to vertical
alignment; that makes the parallelism obvious. Here's another
example:
void pc(pc_type e) {pc1() = 0x02 & e; pc0() = 0x01 & e;}
void rc(rc_type e) {rc1() = 0x02 & e; rc0() = 0x01 & e;}
pc_type pc() const {return pc_type(pc0() + 2 * pc1());}
rc_type rc() const {return rc_type(rc0() + 2 * rc1());}
If there weren't any parallelism, I might be more likely to write
such things on multiple lines.
Another example is given here:
http://lists.gnu.org/archive/html/lmi/2006-09/msg00003.html
Search for "vertically": the simulated errors immediately above
that word are similar to an actual error found in code review of
a proposed patch that used a one-dimensional layout; but the two-
dimensional, vertically-aligned layout makes it easy to spot such
mistakes.
Another similar example:
z = exact_cast<ce_product_name >(m); if(z) return z;
z = exact_cast<datum_string >(m); if(z) return z;
z = exact_cast<mce_basis >(m); if(z) return z;
[...forty-one similar lines...]
z = exact_cast<tnr_unrestricted_double >(m); if(z) return z;
Today, I'd write braces around the returns, but I wouldn't destroy
the two-dimensional layout.