[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] [lmi-commits] master dd5ca93 2/3: Avoid magic values
From: |
Vadim Zeitlin |
Subject: |
Re: [lmi] [lmi-commits] master dd5ca93 2/3: Avoid magic values |
Date: |
Sat, 19 May 2018 14:51:35 +0200 |
On Fri, 18 May 2018 23:10:06 +0000 Greg Chicares <address@hidden> wrote:
GC> On 2018-05-18 17:45, Vadim Zeitlin wrote:
[...]
GC> > I don't necessarily disagree that using 0 width as meaning that the
column
GC> > is hidden was obscure (although it seems like a reasonable extrapolation
of
GC> > the variable value to me...),
GC>
GC> But "0 == width" means "variable width", which also means "clipping".
GC> It's "header.empty()" that means "hidden".
Oops, sorry. I'm afraid I somewhat undermined my own argument here.
GC> > but IMNSHO there should really be readable constants to allow writing
GC> > column_parameters{..., column::visible} instead of the current
GC> > version of the code.
GC>
GC> Do you suggest something more or less like the following?
GC>
GC> struct column_parameters
GC> {
GC> + enum enum_show_or_hide {e_hidden, e_visible};
GC> std::string const header;
GC> std::string const widest_text;
GC> - bool const hidden;
GC> + enum_hidden const hidden;
GC> };
Yes.
Except that this would result in some really long names, i.e. we'd have to
write column_parameters::enum_show_or_hide::e_hidden, so I'd move this enum
to outer scope and maybe abbreviate it to something like "visibility" (why
do we need "enum_" prefix for enums? This looks like Hungarian notation
coming back with the revenge to me...).
GC> If that's what you have in mind, then sure, I'll do it.
Thanks!
VZ