Re: [lmi] sequence input editor -- how to get accepted keywords

[Greg Chicares]

On 2010-05-12 16:11Z, Vaclav Slavik wrote:
> 
> I finally have the input sequences editor in a state when it's able to
> parse string expression, edit it in the GUI and then write the modified
> sequence back into the text field.
> 
> What is missing, aside from some cosmetics, are:
> 
> 1. Good edit window's title. I use "Edit Sequence" now, it should be 
>    something more specific like "Edit payment sequence". This can be
>    specified manually in XRC file as an extra attribute, so unless you
>    disagree with that route, there's no problem.

I don't disagree at all; that's no problem.

Let me take the rest somewhat out of order....

> Also, I use a gross hack to get the Input instance InputSequence needs:
> I find the toplevel window, cast it to MvcController, get MvcModel from
> it and cast it to Input (assuming that InputSequenceEntry control cannot
> occur elsewhere).

We'll want it for class mec_mvc_view too--and for another class very
similar to that one, which I'll be adding later this year. But we can
put that aside and tackle integration with class Input first.

> Any better suggestion would be appreciated; this is
> the best I could untangle the MVC code on my own. Note that this would
> be needed even with 2a), for the automagic duration number change when 
> you change duration mode, as proposed before.

This seems to be the central question. It's really hard for me to answer
without seeing your (unfinished) code. But if that's not ready to share
yet, then I guess a gross hack is probably the best technique for now.

The MVC framework is intended to be encapsulated, so it resists any
attempt to graft on new code from outside--it wants to be extended from
within. But, for a new feature like this one, it may be easier to write
a standalone implementation first, then integrate it into the framework
later.

Before moving on to specifics, let me show how our pre-wx legacy system
handled this:

  void IllusSpecAmtPage::SpecifiedAmountChanged()
  {
    // [...sastrategy_map() provides a list of all keywords possible for this 
field...]
    std::map<std::string, std::string, std::less<std::string> > m = 
sastrategy_map();
    // [...just one example of an enablement conditional...]
    bool prem_indeterminate =
        (
            input_rep.EePmtStrategy != e_pmtinputscalar
        ||  input_rep.ErPmtStrategy != e_pmtinputscalar
        // [...lots more elided here...]
        );
    if(prem_indeterminate)
        {
        // [...adapt to context by deleting "disabled" elements...]
        m.erase("maximum" );
        m.erase("target"  );
        // [...lots more elided here...]
        }

...and then 140 lines later it constructs the InputSequence. That's
exactly what I wanted to avoid when I created the MVC framework for
the port to wx: one event handler per control, with all relevant
knowledge stored in each handler--model knowledge, view knowledge,
and controller knowledge for this control was all combined in this
one function. Then this large function was copied and pasted to
create handlers for all other input-sequence fields. The result was
worse than unmaintainable.

> 2. I need a way to associate a sequence field with the list of accepted
>    keywords (and if there are keywords-only values, a way to know that).

Here are two fields that should accept only keywords:
    datum_sequence           PaymentMode                     ;
    datum_sequence           CorporationPaymentMode          ;
How to know that is part of the general problem discussed below.

>    This would be good for input completion and in-line validation, but
>    most pressingly now, it's required by InputSequence ctor to parse
>    the expression. Right now, I fail to parse "Individual payment" value
>    of "corridor;0" because of this.

'input_realization.cpp' provides functions like this:
  Input::permissible_payment_strategy_keywords()
That's parallel to sastrategy_map() in the legacy example above.
Here's the line that mentions "corridor", for example:
        all_keywords["corridor"] = "PmtCorridor"     ;

This issue appears to have two facets:
 - binding input fields to lists of keywords, and
 - disabling parts of the maps depending on context
but I think we'll see that they ought to be done together.

First, how do we bind these *keywords() functions to the applicable
data members? For example, we want to establish this correspondence:
  Input::PaymentMode --> Input::permissible_payment_mode_keywords()

Today, that's done--unsatisfactorily, I'd say--in 'input_realization.cpp'.
There, Input::RealizeAllSequenceInput() visits each sequence in turn and,
well, "realizes" them all. As mentioned here:
  http://lists.nongnu.org/archive/html/lmi/2010-04/msg00006.html
we could simply call that as an afterthought in Input::DoHarmonize();
but that resembles the one-handler-per-control approach that I argued
against above. Input::DoHarmonize() already has logic like this:

    IndividualPaymentStrategy.allow(mce_pmt_corridor    , !inhibit_prem_simple 
&& !prem_solve || specamt_indeterminate);

but IndividualPaymentStrategy is a *scalar*, whereas we want a sequence.
This conditional code:

  std::string Input::RealizePayment()
  {
      std::map<std::string,std::string> z = 
permissible_payment_strategy_keywords();
      if(mce_solve_ee_prem == SolveType)
          {
          z.clear();
          }

needs to be moved into Input::DoHarmonize(); for that, I think we
should split what Input::RealizePayment() does into two tasks:
 - determine which keywords are permitted in context; and
 - "realize": produce an input_sequence from string input
of which the first certainly belongs elsewhere. (Perhaps we'll find
that we have to do a full "realization" to validate the string.)

Probably the best way is to differentiate all of the numerous data
members that are now of type datum_sequence thus, e.g.:

-    datum_sequence           Payment                         ;
-    datum_sequence           PaymentMode                     ;
-    datum_sequence           CorporationPayment              ;
-    datum_sequence           CorporationPaymentMode          ;
+    payment_sequence         Payment                         ;
+    mode_sequence            PaymentMode                     ;
+    payment_sequence         CorporationPayment              ;
+    mode_sequence            CorporationPaymentMode          ;

for new types as in this very rough sketch:

  class mode_sequence : public datum_sequence {
    // ...forwarding for whatever we need from base class...
    virtual std::map const& keywords
      {...transplant code from Input::RealizePayment() here...}

Of course, today we have only this:
  /// Eventually it may become important to distinguish strings that
  /// represent input sequences, for interactive validation.
  typedef datum_string datum_sequence;
and presumably we'll find good use for a base class there.

>    2a) One solution would be to refactor InputSequence into two classes,
>        one of which would do only parsing, with no checking. But this
>        would complicate errors reporting in there and we would loose the
>        UI goodies listed above.

For those reasons, this doesn't seem like a good solution.

>    2b) We could put this information into the XRC file, but I suspect
>        it must be somewhere there in the sources already, so I'd rather
>        not.

Ah, okay, you just didn't know where it was in lmi. But I guess you
couldn't, because I never ported the parts of the legacy system that
would tie these things together, so you've only got an incomplete
collection of pieces that no longer fit together to make a whole.
In fact, I had to rewrite this message in its entirety because my
first draft pointed you to some legacy vestiges that upon examination
turned out to be dead code. (Don't look in 'input_seq_helpers.?pp'.)

>    Can you think of anything else?

I think defining a UDT for each keyword family is best. XRC might be
okay if the keyword lists were static, but they depend on context.

Actually, at one time I experimented with the idea of writing
Input::DoHarmonize() in Prolog. The problem I though that might solve
is that there are many interdependencies among enablement conditions.
Using a logical inference engine to untangle them is more attractive
than spending time to write imperative-language statements in exactly
the right order. And we could even use this rejected boost library:
  http://www.cc.gatech.edu/~yannis/fc++/boostpaper/fcpp.html
to write Prolog in C++, as discussed here:
  http://www.cc.gatech.edu/~yannis/lc++/paper.pdf
But that would be a separate project.