lmi
[Top][All Lists]
Advanced

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

Re: [lmi] Inversion of control


From: Vadim Zeitlin
Subject: Re: [lmi] Inversion of control
Date: Tue, 18 Sep 2018 03:07:32 +0200

On Tue, 18 Sep 2018 00:37:35 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-09-17 14:15, Vadim Zeitlin wrote:
GC> > On Thu, 13 Sep 2018 00:31:24 +0000 Greg Chicares <address@hidden> wrote:
GC> [...]
GC> > GC> I've written something that does appear to work:
GC> > GC>   http://git.savannah.nongnu.org/cgit/lmi.git/commit/?h=odd/pagination
GC> > GC> but it seems too ugly.
GC> > 
GC> >  The "paginate" class itself looks perfectly fine to me. The only thing 
I'm
GC> > not sure about is whether we really need to separate init() from print(),
GC> > i.e. what's wrong with just passing init() parameters to print() and
GC> > getting rid of the former entirely?
GC> 
GC> It's only a demonstration on a branch, because it's metastable. It wants
GC> to collapse to a simpler state, and there are two ways to do that:
GC>  (A) merge init() into print(); or
GC>  (B) merge init() into the ctor.

 (A) is strictly more flexible than (B) because you can only create an
object once, but you might call print() on it multiple times. For this
reason, (A) would seem to be preferable.

GC> At least when writing C++, I prefer stateful classes whose state is
GC> validly established by constructors

 This is useful if there are non-trivial invariants to preserve. In the
case of print_with_page_breaks() function I don't really see anything like
this. The "state" here consists of the current row and its position and
both are handled very naturally as local variables in this function.

GC> and an imperative flow of control, because it's easier to step through
GC> code when it consists of a sequence of steps.

 I honestly don't see much difference between two approaches from this
point of view. If you use paginator class, you can step into print() and
from there into its virtual method calls. With print_with_page_breaks() you
can step into it and then into the lambdas it calls. It's really not that
different.


GC> In both hierarchies, these entities are often passed as arguments:
GC>         (Ledger const&            ledger
GC>         ,pdf_writer_wx&           writer
GC>         ,html_interpolator const& interpolate_html
GC> and are transmitted from (1) to (2) here:
GC>         for(auto const& i : pages_)
GC>             {
GC>             i->pre_render(ledger_, writer, *this);
GC>             }
GC> and I think I might be able to reduce argument-passing by holding those
GC> entities by reference in the root class of each hierarchy. For me, that
GC> would probably make everything clearer.

 I hesitated to do it like this initially, but decided that it would be
nicer to keep the pages stateless and give them just what they need when
they need it. But I'm fine with this change except that I'd really, really
like to ask you to postpone it until you merge my pagination changes
because there are going to be tons of conflicts between them.


GC> But of course the first thing to be done is the rectification of names
GC> (Confucius, Analects, book XIII, chapter 3), which I'll push soon.

 For me, replacing "regular" with "naic" helps about as much as translating
Confucius back into original, but I'm prepared to suffer for the greater
good. I suspect I'm going to suffer quite a bit when rebasing my changes on
yours however...

 Regards,
VZ


reply via email to

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