
Formal review points: * What is your evaluation of the design? The design is rather complex - it certainly allows great flexibility, at the cost of ready comprehensibility. I realise that the complexity has accrued as a result of the library's extensive 'generalisation' phase; it would be nice to see a brief 'design and evolution' section in the documents. It's a little hard to comment on the design without having participated in the first review; I hope other reviewers can address this. * What is your evaluation of the implementation? The implementation seems quite clean, and well ordered. I think the comments in the code need a review pass, since they are occasionally out-of-sync with the code they comment upon. Also, it would be nice to see a bit more commentary on the purpose of the various classes that make up the implementation, which aren't covered by the documentation. The library includes part of the code as '.ipp' files, for inclusion into user '.cpp' files. This is not a common practise in boost libraries; perhaps it needs to be documented somewhere in the Boost:More Information section. * What is your evaluation of the documentation? The documentation is excellent. The layout is ideal, progressing through tutorial, reference and implementation sections. I think the documentation addresses extending the library with serialization of external types better than it addresses extending the library with new archives (although, this is probably a biased view :) ) I think the 'Advice' section is redundant; these points should be moved to the relevant reference section instead. * What is your evaluation of the potential usefulness of the library? I think the library is potentially of very wide applicability, due largely to its extensibility in the separate domains of types and archives. * Did you try to use the library? With what compiler? Yes, with GCC 3.2.2, without problems. * How much effort did you put into your evaluation? I read the documentation closely, investigated portions of the implementation, and tried to extend the library with types and archives. * Are you knowledgeable about the problem domain? Yes, and no; the library's applicability is broad, so few people will have extensive knowledge of the whole problem domain. I am more familiar with serialization for marshalling than for persistence, or for interoperability. * Do you think the library should be accepted as a Boost library? Yes. I can't think of any major issues that would force further re-design in the library, although new implementation detail issues are surely likely; I think it should be accepted now. Addressing Jeff's questions: 1) This is a large, complex, and important library. Please don't wait till the end of the review period to start your review -- there is no additional room in the review schedule to extend the review period. This is a brief review; please ask if you would like detail on any response. 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. As mentioned earlier, I think that the documentation covers the handling of types better than extension with archives. The section 'New Archives' needs to address the difference between overriding 'save' and 'save_override', and 'operator>>'. Also, discussion of the archive's preamble, and how to disable or modify it seems to be necessary. That said, the library seems not to limit anything an archive implementor may want to do. My only remaining concern is regarding controlling access to indiviual overloads of 'save' or 'load'. This seems to break down, due to the fact that all accesses are from the 'access' class. 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). Not me. 4) Finally, I would like reviewers to indicate parts of the library that might become standalone boost libraries. While this will have little bearing on the acceptance / rejection of the library it might allow a reduction in the size of the library. Some sections of code are definitely candidates for extraction from the serialization library, largely identified by Robert in the 'Miscellaneous' section of the documentation. The strong typedef and state_saver utilities could be extracted, perhaps to the utility library. Perhaps the extended type info infrastructure is useful outside the library. The dataflow iterators are very interesting, but I'm not sure how they interact with the iterator adaptors library, or with sandbox views library. The unicode conversion code is certainly useful, but I imagine it might be better to leave it as an implementation detail of serialization, until someone volunteers to investigate a complete solution for this issue. The code dealing with XML formatting and escaping is in a similar situation. Thanks Robert, for your efforts Matt
participants (1)
-
Matthew Vogt