Re: [lmi] Group premium quotes

[Greg Chicares]

On 2015-06-21 14:44, Vadim Zeitlin wrote:
> On Fri, 19 Jun 2015 23:30:37 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2015-06-19 14:20, Vadim Zeitlin wrote:
[...]
> GC> Input::CorporationName and Input::AgentName, respectively. The
> GC> complication here is that, conceptually, there's
> GC>  - a top level, multiple_cell_document::case_parms_, which should
> GC>    represent input fields that are (again, conceptually) exactly
> GC>    the same for all participants; and
> GC>  - a detail level, multiple_cell_document::cell_parms_, at which
> GC>    every input field is actually present and modifiable (because
> GC>    that makes the system maximally flexible and powerful).
> GC> Here, the end users want the top-level inputs, but they don't
> GC> enter top-level data, and a custom GUI is out of the question
> GC> for this "premium quotes" task.
> 
>  Sorry, I am missing something here: why would a custom UI be needed if,
> AFAICS, both CorporationName and AgentName can already be edited using the
> "Case default dialog" (Ctrl-Shift-E)?

Most end users ignore "Edit case defaults", even though it would be
valuable to them if they gave it a try. This would be an additional
reason for them to learn about it. It's just an education issue.

> GC> I think the best solution is to read these values from first cell
> GC> (participant), and then assert that they match in every other
> GC> cell--assertion failure being an error that prevents any PDF output
> GC> from being generated.
> 
>  Again, I have a strong impression of missing the point here, but it
> doesn't seem right to use an assertion to flag something that is not a
> programmers error: if users can specify different values for these fields
> for the different cells using the "Edit cell" (Ctrl-E) dialog, we should,
> IMHO, display an error telling them that a group summary cannot be
> generated in this case, but why should it be an assertion failure?

You are absolutely right. I wrote "assert" for terseness only.

> GC> >                 ,summary_data
> GC> 
> GC> Input::Comments
> GC> 
> GC> Because this has to be explicitly entered somewhere, I think we
> GC> should prescribe multiple_cell_document::case_parms_ as the place
> GC> where it must be entered: ctrl-shift-E in lmi's GUI.
> 
>  This means that the "Comments" text control would need to be made multi
> line and given some reasonable height, i.e. be made tall enough to contain
> 4-5 lines of text. The problem with this is that it is inevitably going to
> make the dialog bigger and it might not fit 800*600 screens any more. Of
> course, I was the first one to say in the past that 800*600 shouldn't
> matter nowadays and I still believe it doesn't, but I wanted to mention
> this.

It affects only one skin: 'skin_group_carveout.xrc', I think. Personally,
I use only 'skin.xrc'. Other skins can and do exceed my personal limits;
they're designed with the involvement of end users. I don't care if I
can't easily edit these comments myself. I suppose I'll see them as one
long string with some character indicating a line break.

Hey--I just tested that, and to my surprise I got a semicolon delimiter:

/// Paste "\n"- or "\r\n"-delimited clipboard contents into a control,
/// replacing nonterminal delimiters with semicolons to form an input
/// sequence. The motivation is to permit pasting spreadsheet columns.
///
/// At least for now, this transformation is performed iff the paste
/// target is a wxTextCtrl.
void Skeleton::UponPaste(wxClipboardTextEvent& event)

Maybe we'll want to restrict that handler to sequence fields (i.e.,
object class="InputSequenceEntry"), or at least exempt "Comments"
when it has the TE_MULTILINE flag.

>  I also have to admit that I still don't quite understand why can't a new
> field be added to contain this summary data instead of (ab)using comments
> for this. If we could put a multiline "summary fields" text into e.g.
> "Reports" tab of this dialog, it would fit nicely there (currently this
> page is rather underwhelming, compared to most of the other ones)...

That might make sense. First, though, I want to see how the political
discussion in the office shakes out; I'm not going to complicate that
discussion by modifying what we've offered to do, until firm agreement
is reached on the general principles.

> GC> Input::ProjectedSalary, Input::SpecifiedAmount
> GC> 
> GC> Here's a complication that we must sidestep: these are input sequences,
> GC> but we aren't going to "realize" them by calling RealizeProjectedSalary()
> GC> or RealizeSpecifiedAmount(). In the one and only use case, these are
> GC> (string representations of) numeric scalars. Any other use constitutes
> GC> undefined behavior as defined in the language standard, with no diagnostic
> GC> required. If you enter "100000, 3; 50000" then we just copy that string to
> GC> output; or, if you like, we convert it to a numeric scalar, reformat it as
> GC> a string, and no tears if an exception gets thrown for unconvertible 
> input.
> 
>  Is this equivalent to just parsing the first number in this string and
> ignoring the rest? And, if so, what should happen if the first token is not
> a number but one of the keywords ("corridor"...)?

Our emails crossed. You give yet another reason to use class Ledger instead
of treating these complexities as flags in a slalom that, upon reflection,
we needn't ski through at all.

> GC> As an alternative, we might call Input::RealizeAllSequenceInput() and then
> GC> construct a yare_input object and use that. I'll probably do that if it's
> GC> handy for the premium calculations. In that case, I guess we'd just select
> GC> the first element of each input sequence: yare_input::ProjectedSalary[0]
> GC> for example;
> 
>  If this allows to deal with the keywords too, it would seem preferable,
> but I'm not sure at all that I understand this right.

Let's let class Ledger take care of this for us.

> GC> optionally, we could assert that all elements of that vector are equal.
> 
>  And even less sure about what is the right thing to do in this case...

s/assert/print an informative error message if it is not the case/

> GC> Now we could organize the whole either this way:
> GC> 
> GC> void do_everything(...filename...)
> GC> {
> GC>    intermediate_struct z = foo(filename);
> GC>    // total every column somewhere...
> GC>    do_pdf_stuff(z);
> GC> }
> GC> 
> GC> or by intermingling foo() with PDF stuff in a single function. Is this
> GC> concrete enough for you to see which way we should do this so that we
> GC> don't wind up building a "Psychobilly Cadillac"?
> 
>  I prefer to have an intermediate struct and do_everything() function. The
> only drawback of this approach I can see is that copying data around is not
> as efficient as using it directly, but I don't believe we're going to have
> performance problems because of this, we just don't operate with that much
> data here.

Your intermediate struct is class Ledger. I think it's available as a
boost::shared_ptr already; if not, then at least its heavy components are,
so we can copy this big dataset around without worrying about performance.

class LMI_SO Ledger
{
... for example ...
    boost::shared_ptr<LedgerInvariant>   ledger_invariant_;

>  Other than this I can see only advantages in using this struct:
> 
> - The function building it can be tested independently, we don't need to
>   dump the data to any ostringstream inside the function itself which might
>   be cheap but is not free and also distracts from the main point of the
>   function. If it just returns the struct, it can be dumped/verified in the
>   test code.

+1 for reusing the existing class Ledger

> - Similarly, the function producing the PDF can be tested much easier as we
>   don't need to rewrite any driving function in the test, instead we can
>   just populate the struct (which is much simpler) and pass it on.

+1 for reusing the existing class Ledger

> - There are fewer constraints over both the struct creating and the PDF
>   generating code if they can write/read this struct in any way they want
>   instead of having to do it row by row. E.g. the PDF function might look
>   at the total number of rows to determine the total table height and the
>   number of pages (this is just an example, I don't think we need to do it
>   right now, but it's still good to have the possibility to do it if
>   needed). Similarly, perhaps the creating function might be more naturally
>   written if it computed the basic premiums for all rows first and then the
>   other ones, instead of having to do it strictly row by row.

I guess it's kind of like DOM vs. SAX.

The existing "Census | Print roster" command creates a TSV file presupposing
a single "spreadsheet" page of arbitrary length. Pagination makes this PDF
file a harder problem, which benefits from having all data in advance.

On the other hand, let's be sure this thing works with, say, ten thousand
cells in the census. I recall one case that abended after about two thousand
due to msw resource exhaustion; IIRC, it ran out of file handles that were
kept open when illustrations were run in a particular way. That probably
won't be a problem here, but let's check it early in development.

>  So I (still) think that using an intermediate struct is the way to go.

+999 for reusing the existing class Ledger, then; we have a winner.