Re: [lmi] Why use pointers here?

[Vadim Zeitlin]

On Mon, 2 Apr 2018 13:19:49 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-03-31 16:18, Vadim Zeitlin wrote:
GC> > 
GC> >  [...] please see https://github.com/vadz/lmi/pull/78
GC> BTW, why use pointers here?
GC> 
GC> @@ -2942,16 +2929,16 @@ void ledger_pdf_generator_wx::write
GC>      switch(z)
GC>          {
GC>          case mce_ill_reg:
GC> -            pdf_ill = std::make_unique<pdf_illustration_regular>(ledger, 
output);
GC> +            pdf_ill = std::make_unique<pdf_illustration_regular>(ledger);
GC>              break;
GC> ...
GC> -    pdf_ill->render_all();
GC> +    pdf_ill->render_all(output);
GC> 
GC> AFAICT, classes like pdf_illustration_regular aren't "factories"
GC> that only return smart pointers, so couldn't we instead write:
GC> 
GC>   pdf_illustration_regular>(ledger).render_all(output)
GC> 
GC> and similarly for the other classes?

 This would require repeating the render_all() call for all the 4 cases,
instead of doing it only once or hiding it inside pdf_illustration_xxx
itself (calling it automatically from dtor, perhaps?[*]), which would be
less clear IMO. But maybe what we really want is a helper function like
this:

template<typename T>
void create_illustration(Ledger const& ledger, fs::path const& output)
{
    T pdf_ill{ledger};
    pdf_ill.render_all(output);
}

that would be called from the cases in this switch? I hesitated creating
a function just for this, but thinking more about it now, I think it
would really be a better idea than using a pointer unnecessarily.

 Should I make the (trivial) patch changing the code like this?
VZ

[*] I should have totally written the proposal to do it yesterday, sorry
    for being a bit late.