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

[Greg Chicares]

On 2010-06-27 11:02Z, Vaclav Slavik wrote:
> On Sun, 2010-06-27 at 08:15 +0000, Greg Chicares wrote:
>> > In 'datum_sequence.hpp', are the value_cast<>() specializations
>> necessary?
>> 
>> For derived classes (payment_sequence and mode_sequence), I mean. 
> 
> It seems you're right and they aren't needed -- I don't get runtime
> errors in the place I tested for while writing the code (i.e.: click
> "..." on the Individual payment field).

I commented out the derived-class specializations, and tried to make
a failing unit test. I thought this would fail:

    payment_sequence p;
    std::string sp = "1;0";
    p = value_cast<payment_sequence>(sp);
    sp = value_cast<std::string>(p);
    BOOST_TEST_EQUAL("1;0", sp);
    BOOST_TEST_EQUAL(false, p.keywords_only()); // must be 'false'

    mode_sequence m;
... similar to the above
    BOOST_TEST_EQUAL(true, m.keywords_only());  // must be 'true'

because keywords_only() has to give different answers for these
two types. But the tests pass because I coded the type explicitly:
    p = value_cast<payment_sequence>(sp);
I still fear that such a test would fail if we transfer data between
strings and the (soon to be) abstract base class datum_sequence.
If that doesn't fail, then I can't explain why it wouldn't.

But the reason I started thinking about this is that I feel I've been
specializing value_cast<> way too often recently (for xml product data),
and it feels wrong to do that for a significant number of new types.
However, I think I've found a much better way--add this:

inline std::istream& operator>>(std::istream& is, datum_string& z)
  {z.read(is); return is;}
inline std::ostream& operator<<(std::ostream& os, datum_string const& z)
  {z.write(os); return os;}

in 'datum_string.hpp' and remove the value_cast specialization for
every class that derives from it, directly or indirectly. Then
value_cast<> just does the right thing (by invoking stream_cast<>).

> I think I copied it from other code without realizing it is not
> necessary,

Specializing for datum_sequence (as well as its base class) but
not for classes derived from datum_sequence is just a weird idea
that I stumbled upon by accident. Even if it's completely valid
and robust (which I doubt), I'd feel compelled to document it
carefully, always fearing fragility. It would be easier and safer
to leave it just the way you wrote it, so that it works obviously.
You cloned the other code correctly; it's just that I've grown
allergic to what that other code does--so it's the old code that
I want to change, in order to make the new code simpler.

> unlike other pieces such as reconstitutor<> changes (I'm not
> entirely clear on details of the inner workings of LMI's model code yet,
> so it's entirely possible I made other similar mistakes).

I was going to ask why 'input.hpp' has a separate reconstitutor
for class datum_sequence:
  template<> struct reconstitutor<datum_base, Input>     // old
  template<> struct reconstitutor<datum_sequence, Input> // new--needed?
but now I guess there was no subtle and profound reason: that's
all I need to know. First we get it correct, then we refactor
into simplest form.