[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Why use pointers here?

From: Vadim Zeitlin
Subject: Re: [lmi] Why use pointers here?
Date: Mon, 2 Apr 2018 15:27:17 +0200

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> @@ -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, 
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> AFAICT, classes like pdf_illustration_regular aren't "factories"
GC> that only return smart pointers, so couldn't we instead write:
GC>   pdf_illustration_regular>(ledger).render_all(output)
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

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

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?

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]