Re: [lmi] Inversion of control

[Greg Chicares]

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

It's only a demonstration on a branch, because it's metastable. It wants
to collapse to a simpler state, and there are two ways to do that:
 (A) merge init() into print(); or
 (B) merge init() into the ctor.

> GC> ...because I don't see how I can implement the pagination class's virtuals
> GC> as members of class page_with_tabular_report. The reason is that the
> GC> tabular-report class has only member functions--it doesn't have data 
> members,
> GC> so AFAICT an instance has no state.
> 
>  But what exactly prevents us from adding it? It's true that originally the
> page classes were not supposed to have any state, but it was only because I
> thought it would make them simpler and not because if any overarching
> philosophy. If it's now simpler to store some state in them, I see nothing
> wrong with doing it.

Thanks, that's exactly what I needed to know: I can use (B) above.

And then on my odd/pagination branch
  git show 91ddea1fdac
I'll probably fold the first of these classes into the second:
  class paginate
  class paginator
and use it as a mixin.

>  In the "functional style" you don't have a virtual print_a_data_row() to
> override in the first place. Instead, you have just a print() function,
> which takes the same parameters as paginate::init() takes now as well as a
> number of callbacks corresponding to the different virtual methods.

[...snip lambdas...]

> Note that this requires passing position and year as parameter to the
> functions and returning the new position from them because they, being
> functions and not objects, can't have any state now. But IMHO this is not a
> drawback in this case as the state is simple and it's easy to manage it
> inside print_with_page_breaks() itself rather than in some object
> (functional programming fans would tell you that it's never a drawback, but
> I'd be content with a less sweeping statement).

At least when writing C++, I prefer stateful classes whose state is
validly established by constructors--and an imperative flow of control,
because it's easier to step through code when it consists of a sequence
of steps.

I see two main hierarchies in 'ledger_pdf_generator_wx.cpp':

(1) Whole illustrations: ledger_pdf_generator_wx::write() creates one of
these leaf classes [inheritance indicated by progressive indentation]...

class html_interpolator
 class pdf_illustration : protected html_interpolator
    class pdf_illustration_regular : public pdf_illustration [has ctor]
    class pdf_illustration_finra : public pdf_illustration    [has ctor]
    class pdf_illustration_reg_d_group : public pdf_illustration [has ctor]
    class pdf_illustration_reg_d_individual : public pdf_illustration [has ctor]

(2) Components that are composed via add<>() to create classes in (1)...

class page
  class cover_page : public page
  class page_with_footer : public page
    class numbered_page : public page_with_footer [has ctor]
      class standard_page : public numbered_page [has ctor]
        class ill_reg_numeric_summary_page : public standard_page
          class ill_reg_numeric_summary_attachment : public 
ill_reg_numeric_summary_page
    class page_with_tabular_report : public numbered_page, protected 
using_illustration_table
      class page_with_basic_tabular_report : public page_with_tabular_report
        class finra_basic : public page_with_basic_tabular_report
        class reg_d_group_basic : public page_with_basic_tabular_report
      class ill_reg_tabular_detail_page : public page_with_tabular_report
      class ill_reg_tabular_detail2_page : public page_with_tabular_report
      class finra_assumption_detail : public page_with_tabular_report
      class reg_d_individual_irr_base : public page_with_tabular_report
        class reg_d_individual_guar_irr : public reg_d_individual_irr_base
        class reg_d_individual_curr_irr : public reg_d_individual_irr_base
      class reg_d_individual_curr : public page_with_tabular_report
      class standard_supplemental_report : public page_with_tabular_report
              [has ctor with an html_interpolator const& argument]
        class ill_reg_supplemental_report : public standard_supplemental_report

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

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