On Apr 12, 2004, at 10:09 PM, Jeff Garland wrote:
2) I am particularly interested in hearing from parties that have tried to extend the library by adding serialization for a new type or by building a new type of archive. The ability to easily extend the library for new types and novel types of archives is critical for this library.
I have tried extending the library since I want to swap our old serialization library used at http://alps.comp-phys.org with a Boost serialization library as soon as possible. It was important to keep the same archive format, since we have gigabytes of data collected over ten years in millions of CPU-hours and need to be able to read those with both new and old codes. With the help of Robert I managed to convince myself that this is indeed possible. The only issue I have is that the documentation needs to be improved.
3) If you reviewed the library during the first review, I would like your evaluation of whether the new version of the library addresses your original concerns. Also, if this is a re-review I would appreciate if you could send me a weblink from the mail archive to your original review posting(s).
In the first round I voted against inclusion in Boost because i) the design was not flexible enough to allow reading my old legacy formats and ii) the design inhibted some optimizations I need for large data sets. Robert did an excellent job in addressing this and other issues. The new version is greatly improved and I am satisfied with the design.
As usual, please state in review comments how you reviewed the library and whether the you think the library should be accepted into Boost. Further guidelines for writing reviews can be found on the website at:
http://www.boost.org/more/formal_review_process.htm#Comments
What is your evaluation of the design?
The current design offers the flexibility needed for all serialization tasks I can think of and makes it possible to implement serialization without any performance penalty.
What is your evaluation of the implementation?
I did not look at many details of the implementation but have a few comments: 1. there are still a few "dangerous" code portions, such as load_binary(const_cast<char *>(s.data()), l); in line 89 of file basic_binary_iprimitive.ipp . It is stated in a comment that this might be unsafe, but it is probably OK on all current implementations of the standard. 2. the comments in the source files are at several places out of sync with the implementation, they sometimes still refer to old designed of the library. A thorough check of all files is needed. 3. looking at the implementation after reading the documentation is confusing at some places because of tricks used to work around compiler deficiencies. For example, the documentation discusses load() and save() functions but the implementations use load_override() and save_override() functions. I would prefer to have the implementation be consistent with the documentation and the workarounds included only in #if .... #endif guards.
What is your evaluation of the documentation?
The documentation is OK for using the library, but more and better documentation is needed for: 1. implementing new archive types: there, as Robert is aware, the example in the documentation is incorrect. A more extended documentation would be very useful. 2. Better documentation for the classes used to track classes, class version, objects, etc.. For example, in newarchive.html there is a table listing some types (class_id_optional_type is listed twice???) but I am missing a) more information about the purpose of these types b) I found some additional types such as class_name_type, which is used in the implementation of some archives but not documented. 3. Documentation for the workarounds used to placate deficient compilers (e.g. save_override, ...) is missing 4. Documentation of archive formats, especially what class and object information is stored. I want to be able to predict, from reading the documentation, what exactly will be written to the archive and in which order. This is essential when exchanging data with an application that does not use this library.
What is your evaluation of the potential usefulness of the library?
In my opinion this will be among the most useful and most widely Boost libraries.
Did you try to use the library? With what compiler? Did you have any problems?
I tried using the library on MacOS X 10.3 using Apple's version of the g++ 3.3 compiler. I encountered two problems. 1. archive /wcslen.hpp does not compile because for some unknown reason boost/config.hpp defines BOOST_NO_CWCHAR, although the <cwchar> header exists. Is there a deep reason for this configuration option on MacOS X or is this a bug in the configuration? 2. trying to compile text_wiarchive.cpp and xml_wiarchive.cpp I get error messages such as boost/archive/iterators/remove_whitespace.hpp:146: error: ` boost::archive::iterators::filter_iterator<Predicate, Base>::m_predicate' has incomplete type I did not investigate this further at this time because of lack of time. and because I do not currently need the wide stream archive types.
How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Being busy and traveling from Switzerland to California has not allowed me time for an in-depth study of all aspects. I focused compiling the library under MacOS X, writing short example programs to studdy the archive formats with, and implementing a new archive type.
Are you knowledgeable about the problem domain?
I have use serialization in a portable binary format on a daily basis for a decade. To summarize, I vote for accepting the library into Boost but would like to see the documentation improved. Matthias