[boost] FW: Serialisation library review

From: Robert Ramey (ramey_at_[hidden])
Date: 2002-11-23 00:16:23

Note:  This was sent directly to me by Pavel Vozenilek.  I am posting it to 
the list.

Robert Ramey

I recommend to accept serialisation library into Boost.

I played with the library for few hours and used Intel C++ 6.0
plugged in Visual C++ 6.0 IDE (and Visual C++ 6.0 STL) to compile
examples and debug the code. I also read the documentation available.

I like the library interface: within C++ limits it is
minimal and natural. I had coded serialisation code few times
in similar way (and didn't like to write the infrastructure over
and over).

The implementation needs a quite lot of effort to understand: probably
more comments on internal structures would help. From pragmatic point
of view I see this as the smallest problem.

The documentation is IMHO the weakest point. Serialisation probably
isn't highest priority for application programmer and lack of easily
understandable documentation can drive him/her of this library
to some ad-hoc quick and dirty code.

I recommend:
- complete redo of the documentation (the text should
  be made more clear on many places),
- many more of code examples in documentation
  (and these being complete and compilable),
- the code examples should have different background,
  e.g. like it is in Spirit library documentation,
  it is then much easier to read them,
- reference documentation should be broken into few parts,

More issues are in attached text file. None of them, however
looks to me as showstopper.

/Pavel

_________________________________________________________________
Help STOP SPAM with the new MSN 8 and get 2 months FREE*  
http://join.msn.com/?page=features/junkmail
 


----------------------------------------------------------------------
MAJOR ISSUES:

1) Is there any exception for disk-full error? Also exception for
   invalid state stream should exists.

2) function text_archive.init() should not call seekp(0).
   The stream may contain other data that would be overwritten.

3) This is a bit nitpicking: volatile members/pointers/classes may be
   also serialised. Currently it doesn't work.

4) I could not make demo.cpp working: there's some problem with
   is_polymorphic<>. I will be glad to provide more info if asked.

5) function get_guid() isn't mentioned in the documentation.

----------------------------------------------------------------------
MINOR ISSUES:

1) Is there any possibility to plug-in compression of data (or make
   archive subclass for it)? If, can it be documented with small example?

2) Likewise, is there any chance to plug-in OS specific file routines?
   (e.g. WIN32 non-buffered reads and/or scattered reads/writes).
   This may be important for performance reasons.

3) If exception happens during writing data to file,
   shouldn't the stream be rewinded back to original position?

4) set<>, multiset<>, hash_set<> and hash_multiset<> don't seem to be
   included in stl.hpp. Also maybe rope<> ('big string' from SGI STL)
   can be included.

5) The function text_archive::newtoken(): does it really need to be
   virtual function? It is pretty big overhead to call it for each
   member field.

----------------------------------------------------------------------
CONSISTENCY CHECK SUPPORT:
(manually written serialisation code can be very dangerous during 
maintenance)

1) Can consistency between save() and load() functions supported?
   I mean e.g. number of items saved/loaded needs to be the same,
   their names/types the same etc.

   Maybe if there is macro or runtime parameter to enable/disable
   this type of check.

2) Can we have some function returning size of data being written
   for an object? It could be used for checking, like:

      void AClass::save(...) {
        ar << field1; ar << field2; ...
        assert(sizeof(AClass) == ar.writtenBytes()); // check
     }

   (Such a check is applicable only under circumstances.)

3) Can we have save()/load() functionality merged together not to duplicate 
code
   and make hand-written code safer?

   Something like:
   void load_save(..., bool is_saving) {
     ar.load_or_save(field1, is_saving);
     ar.load_or_save(field2, is_saving);
   }

  Maybe such a functionality can be provided in addition to current code.

4) Can there be library version (maybe enabled/disabled by macro) that
   doesn't require (and doesn't store) version()? This may be of use
   for high performance stable systems.

5) Can the call to base class save()/load() from derived save()/load() be
   recognized (and assert()ed) in debug version?

    (As opposite to valid call base_object<>().)

----------------------------------------------------------------------
DOCUMENTATION ISSUES:

1) Can space requirements be documented for ASCII/UNICODE/binary
   archiving? Can it be documented on example?

2) First example in tutorial.html includes <iostream> twice.
   Also "using namespace std;" is missing. "ofs.close()" should be
   replaces by "ifs.close()", "ar" by "ia".

   Also information what *.cpp file to add to the project is missing
   (archive.cpp, void_cast.cpp and serialisation.cpp).

   (Also BOOST_NO_STD_WSTRING and BOOST_NO_INT64_T needed to be defined
   to correctly compile the code. This may be specific to combination
   Intel C++ 6.0 and Visual C++ 6.0 library.)

   Since this is first code one may try, it should work out of the box.

3) Can references by saved/loaded? (I assume not but didn't find in 
documentation.)

4) Typo in tutorial.html: "is found int serialization.hpp". (the 'int')

5) Typo in reference.html: "will invoke a compiler time assertion".

6) The documentation (in reference.html) for
   "A pointer to a primitive type cannot be serialized" sentence
   should include hint what to do with such a case.

7) Maybe there should be tutorial page how to write serialisation support
   for some non-standard container.

8) What is the meaning of "Attempting to save a non-const variable will
   invoke a compile time assertion"? (in reference.html). I don't get it.

   Could each advice be accompanied by short code sample?

9) The tests should be in separate directory.

10) It should be stated Boost 1.29 is required (because of MPL).

11) register_type<> is not documented. I feel the whole section on
    polymorphic classes is unclear and should be redone.

12) The docs doesn't say if the serialised data can be transmitted over wire
    to another application (I assume this is NOT purpose of this lib).

13) The description of example for "Very Large Numbers of Small Objects"
    section isn't clear to me.

14) register_type<> versus virtual load()/save(): which can be used when,
    what are advantages/disadvantages? I do not understand term "seen"
    used here. Can some simple example be used?

15) Some performance benchmark would would be nice. Also info where was
    this library ported so far.

EOF