Re: [lmi] Pagination anomaly

[Vadim Zeitlin]

On Tue, 6 Feb 2018 20:25:54 +0000 Greg Chicares <address@hidden> wrote:

GC> Agreed, but now that I've sunk my teeth into it...sometimes all that
GC> titanium just has a mind of its own.

 This is unfair. How am I supposed to explain why do I regularly do the
same thing even without any titanium in my body? In any case, good luck
with this change, if my own experience is any guide, such incremental
improvement changes often end up being much more interesting than initially
foreseen...

GC> BTW...I know I'm supposed to have moved on to reviewing something
GC> else now, but in 'group_quote_pdf_gen_wx.cpp':
GC> 
GC>     for(int col = 0; col < e_col_max; ++col)
GC>         {
GC>         column_definition const& cd = column_definitions[col];
GC>         std::string header;
GC> 
GC>         // The cast is only used to ensure that if any new elements are 
added
GC>         // to the enum, the compiler would warn about their values not being
GC>         // present in this switch.
GC>         switch(static_cast<enum_group_quote_columns>(col))
GC> 
GC> wouldn't it be better if 'col' were of type 'enum_group_quote_columns'?

 I'm not sure if it's really better. It saves the cast here, but it
requires doing something ugly when incrementing it.

 Also, this code predates the switch to C++11 in lmi, but nowadays I would
have used "enum class enum_group_quote_columns" and then using proper type
for this variable would require many more casts, i.e. would be even worse.

GC> I'm guessing you would have made it such, if only it were easy to iterate
GC> across all enumerators. I'd like to attempt such a change, at least to
GC> see if it looks any better; is defining an array that includes each
GC> enumerative value, and using ranged for, still the least awful way of
GC> doing this?

 AFAIK, yes. And this one at least has the advantage of working with scoped
C++11 enums as well, unlike the solution with the cast to int for
increment.

 Regards,
VZ