lmi
[Top][All Lists]
Advanced

[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.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]