Re: [lmi] Group quotes, part deux

[Greg Chicares]

On 2016-05-11 13:01, Vadim Zeitlin wrote:
> On Sun, 8 May 2016 22:16:25 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> > GC> Later, in output_document_header(), I have a different design 
> question:
> GC> > ...
> GC> > GC> Is there good reason not to treat everything the same way, e.g.,
> GC> > GC> replacing the above block by
> GC> > GC> 
> GC> > GC>     fields.push_back(extra_summary_field({"Plan Type", 
> report_data_.product_}));
> GC> > GC> +   fields.push_back(extra_summary_field({"Product", plan_type_}));
> GC> > GC> +   fields.push_back(extra_summary_field({"Effective Date", 
> report_data_.effective_date_}));
> GC> > GC> 
> GC> > GC> and letting the loop for "parsed" input handle everything? I don't
> GC> > GC> want to stomp on your design without asking, lest I destroy 
> something
> GC> > GC> valuable that I just don't happen to notice.
> GC> > 
> GC> >  No, I don't think this was done intentionally at all, AFAIR the extra
> GC> > summary fields were added later and I just didn't realize I could 
> refactor
> GC> > the existing code to handle both the built-in and the extra fields in 
> the
> GC> > same way. Looking at it right now, this definitely looks like a good 
> idea.
> GC> > Should I do it or leave it to you as you're already in this code?
> GC> 
> GC> Please do it, as you're the original author so it's easier for you.
> GC> I've completed the changes I intended for now, so I'll hand this
> GC> source file over to you.
> 
>  I'll do this slightly later, for now I'd really like to deal with the two
> questions above.
[...]
> GC> ...so let me ask you to address this globally as you like along
> GC> with any other C++11-ization that seems advisable.
> 
>  As mentioned before, I'd prefer to modernize the entire code base at once,
> as otherwise it's going to be jarring to see "NULL" here and "nullptr"
> there, lmi::uncopyable<> here and "= delete" there and so on. But doing it
> right now would certainly create many conflicts with the still outstanding
> patches, so I'd like to wait until you can apply at least those of them
> that you plan to apply.

Would you mind addressing this particular part now, separately from the
global modernization? Here's why I ask. This code in
  group_quote_pdf_generator_wx::output_document_header()
already uses emplace_back(), along with two other methods to assemble the
same set of headers, so this isn't just modernization--it's restructuring
for consistency and simplicity. And we're testing group quotes thoroughly
now, so testing this change separately later will take extra effort that
can be avoided by doing it now.