Re: [lmi] Preliminary emit_ledger code refactoring

[Greg Chicares]

On 2015-07-16 20:41, Vadim Zeitlin wrote:
[...]
>       Use ledger_emitter class instead of [pre_]emit_ledger() functions.

I'm studying this patchset now, and I'm stuck on this particular patch.

Previously the ledger-emitter implementation was just two functions:
  double LMI_SO pre_emit_ledger(...);
  double LMI_SO     emit_ledger(...);

It might seem sensible enough to combine those into one single class
  class LMI_SO ledger_emitter
because, after all, some of the arguments of those two functions were
the same and may as well be stored as members.

OTOH, this patch adds
  class ledger_single_format_emitter
with derived classes
  class ledger_emitter_pdf_file       :public ledger_single_format_emitter
  class ledger_emitter_pdf_to_printer :public ledger_single_format_emitter
  class ledger_emitter_pdf_to_viewer  :public ledger_single_format_emitter
  class ledger_emitter_test_data      :public ledger_single_format_emitter
  class ledger_emitter_text_stream    :public ledger_single_format_emitter
  class ledger_emitter_custom_0       :public ledger_single_format_emitter
  class ledger_emitter_custom_1       :public ledger_single_format_emitter
and additionally
  class ledger_emitter_single_output_base :public ledger_single_format_emitter
with its derived classes
  class ledger_emitter_spreadsheet    :public ledger_emitter_single_output_base
  class ledger_emitter_group_roster   :public ledger_emitter_single_output_base
and further
  class ledger_emitter_impl
all of which makes it much more complex.

Maybe that's in the eye of the beholder. Maybe this is a more modern way of
doing things, and I've simply fallen behind the times. Maybe this is clearer
from a functional-programming point of view, while I'm clinging to a 1950s
imperative style and understand C control structures more readily than class
hierarchies. Whatever the reason, I've tried pretty hard to like this, but
I'm stuck because, to me, this way seems harder to understand. So I'd like
to consider whether something simpler could achieve the objective, which
seems to be only this:

>       Use of the class allows to preserve internal state between the calls to
>       pre_emit_ledger() and emit_ledger() which will be important for the 
> premiums
>       PDF emission to be added soon.

as explained in more detail here:

> They are needed because the
> PDF generation can't be done in a single pass, if only because of the page
> numbers (but also for a few other reasons), and so the code needs to keep
> its state between the calls to emit_ledger().

How big and complicated is that state? Is it the entire premium-quote-pdf
implementation, or just some small part of that? I'd guess that it's not
simple, just from my limited experience with trying to use html for a
comparable purpose--in order to say "page M of N" on each page, you need
to know N, which means you have to lay out the whole thing and then go
back and plug N in. Of course, in that example, if a single integer N is
the only state that needs to be preserved, then that's quite simple; but
I doubt the present case is that trivial.

Could this state be stored elsewhere?

Alternatively, could we use emit_ledger() to write a TSV file--much like
the present PrintRosterTabDelimited() but with different contents
distilled out of the ledger objects--and generate the PDF from that file?

> It also needs to know when
> the last ledger is added, and while it could be done by adding a
> post_emit_ledger() function or even just supposing that the call to
> emit_ledger() with a composite ledger indicates the end of the generation,
> adding a class with a separate end() method looked like the most natural
> solution to me.

The (unpatched) emit_ledger() can be used for a single_cell_document (.ill),
in which case there's no composite. But for premium quotes it can be used
only for a multiple_cell_document (.cns), in which case the composite by
its nature is always the last ledger generated--that's required by the
problem domain.