lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] change file formats to XML


From: Vaclav Slavik
Subject: Re: [lmi] change file formats to XML
Date: Tue, 16 Mar 2010 14:29:12 +0100

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.

>  - Somehow I had difficulty understanding the use of the convenience
>    wrappers within the 'xml_serialize.hpp' implementation itself.

The only reason to use them was that they handled choose_type_io<>
indirection. With that taken care of by the point above, there's no need
for their use within xml_serialize.hpp.

>  - As for containers [I'm sure the specialization given works for
>    sequences, but I'm not sure about other containers]...

"Sequence" was the work I was looking for, not "container", thanks for
fixing that.

>    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.

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.

Regards,
Vaclav





reply via email to

[Prev in Thread] Current Thread [Next in Thread]