Re: [lmi] [lmi-commits] master 0d3c75a 5/6: Improve readability

[Vadim Zeitlin]

On Mon, 13 Mar 2017 01:33:33 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2017-03-12 23:33, Vadim Zeitlin wrote:
GC> > On Sun, 12 Mar 2017 23:03:44 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > GC> On 2017-03-03 14:37, Vadim Zeitlin wrote:
GC> [...]
GC> > GC> Yes, its sole purpose is to break up a line which would otherwise
GC> > GC> be too long to fit on a single card:
GC> > GC> 
GC> > GC> 
00000000011111111112222222222333333333344444444445555555555666666666677777777778
GC> > GC> 
12345678901234567890123456789012345678901234567890123456789012345678901234567890
GC> > GC>         std::string z = 
value_cast<std::string>(duration_num_field(i).GetValue());
GC> > 
GC> >  What's wrong with
GC> > 
GC> >             std::string const z = value_cast<std::string>
GC> >                             (duration_num_field(i).GetValue()
GC> >                             );
GC> 
GC> That's three whole lines: one-eighth of my screen, which only displays
GC> twenty-four card images at a time. The original is only two lines:
GC> 
GC>         auto const endpoint = duration_num_field(i).GetValue();
GC>         std::string const z = value_cast<std::string>(endpoint);

 I can understand this argument, but I have to really wonder why you don't
use K&R indentation conventions as the gain from writing

        if (cond) {
                ...
        } else {
                ...
        }

compared to the current lmi style is even greater than this. Vertical
compactness is one of the things I really like about this style. But this
is an idle question, of course, I don't propose reformatting all lmi
code...


GC> > GC> whereas I look at the code as though this patch had been applied:
GC> > [...patch compactifying the switch snipped...]
GC> > 
GC> >  OK, this does make things better.
GC> 
GC> Would you actually recommend applying a patch such as that, especially
GC> given that the unchanged lines would be irregular?

 I don't know :-( I started writing a positive reply, realized that I
didn't really like the resulting irregularity, so started writing a
negative one, but realized that I really didn't like using an "unnamed"
variable so far (20 lines) after its definition neither. If we could make
an exception and omit the lines containing only braces in this switch, it
would IMHO be ideal as it would still be regular, but compact enough.

GC> (I really like condensed, table-style code; maybe there's a way to
GC> force regularity upon the other cases.)

 I don't really see how...


GC> I run every automated test I know before each push. This was too tiny to
GC> push alone, when I had other things pending. I've popped enough off the
GC> bottom of my email queue that your 64-bit patch is next....

 Thanks for applying it and my other patches!
VZ