Re: [lmi] change file formats to XML

[Greg Chicares]

On 2010-03-16 13:29Z, Vaclav Slavik wrote:
> On Tue, 2010-03-16 at 04:07 +0000, Greg Chicares wrote:
>>  - For mc_enum types, I thought it better to use the (heavy) mc_enum
>>      instead of the underlying (light) C enum; that doesn't add much
>>      overhead in 'ihs_rnddata.cpp', and it writes (clear) names in
>>      the xml instead of (obscure) integers, e.g.:
>> 
>>       <specamt>
>>         <decimals>0</decimals>
>>         <style>Upward</style>
>>       </specamt>
>> 
>>      while simplifying 'xml_serialize.hpp'.
> 
> It would be worth to add an assert to xml_io<T> that verifies that T is
> not an (light) enum, then. If I understand value_cast<> correctly, this
> will simply serialize any enum as an integer, won't it? And that's
> something we never want.

Agreed: I'll add that later. (Right now I'm working in a different tree
and can't conveniently test such a change.)

>>    an asymmetry that troubled me: from_xml() called clear() on the
>>    container, but to_xml() didn't do anything similar. I know you
>>    had to do this in order to keep your patch simple by avoiding
>>    major changes to other code, 
> 
> Actually, I did that for reasons entirely independent of anything
> outside of xml_serialize:
> 
> The generic from_xml() implementation _overwrites_ (or, to use another
> word, "sets") the value on read, that's its public interface. But
> xml_sequence_io without a clear() call _appends_ deserialized data to
> existing value. Same method, different semantics.

That was another asymmetry that troubled me:
  get_property() vs. add_property()
But maybe my approach was somewhat procrustean:
  get_element()  vs. set_element()
and naming two things so that they seem symmetric doesn't make them so.

I was thinking of a default '.xrnd' file (for example) as containing:
  <?xml version="1.0"?>
  <rounding>
    <specamt>
      <decimals>0</decimals>
      <style>whatever-the-default-ctor-would-do</style>
    </specamt>
as though it were a C struct that can't fail to contain values; however,
your view that a default xml document is empty seems a lot more correct.

> I thought it would be unnecessary to clear output element's children in
> to_xml(), because we have greater control over the 'e' argument (I
> should have made than an explicit -- and checked for -- prerequisite).
> Unlike that, T can be any type used in the app and we can't very well
> document from_xml() to "append deserialized value to 't'". It wouldn't
> make sense for many times (think T=int or T=bool).
> 
> So I think it would be better to keep the semantics of reading the
> entire value in from_xml() and call clear() in the xml_sequence_io
> version.
> 
> As for the asymmetry with to_xml(), adding the following to clear the
> node would fix it:
> 
>    e.erase(e.begin(), e.end());
>  
> If 'e' is empty (as it is), it's a cheap thing to do. Or I could add
> xml::node::clear() to xmlwrapp for convenience.

Well, I guess we either
 - name functions as though they were symmetrical, and document why
     they actually aren't; or
 - name functions to reflect the inherent asymmetry, and document that.
But right now I'm a little confused, and I don't want to make another
bad guess as to what you intend, so could I ask for a tiny patch against
HEAD? I think it's enough just to say what names you'd use for these two
functions and whether you'd use assertions or call erase() (or clear()),
nothing more than a sketch like this:

  struct xml_sequence_io
  {
  ...
    static void to_xml(xml::element& e, T const& t)
    {
-       LMI_ASSERT(e.elements("item").empty());
+       e.erase(e.begin(), e.end());
  ...
    static void to_xml(xml::element& e, T const& t)
    {
+       t.clear();
-       LMI_ASSERT(t.empty());
  ...
  template<typename T>
- void set_element(xml::element& parent, std::string const& name, T const& t)
+ void add_element(xml::element& parent, std::string const& name, T const& t)
  ...
  void get_element(xml::element const& parent, std::string const& name, T& t)

and then I can figure out how to do the global replacements myself.
I don't need something that compiles--I just want to get your ideas
with minimal effort on your part.