
Here's my review: ***** What is your evaluation of the design? ***** The overall design seems sound. However, apart from what other reviewers mentioned I have a few additional suggestions and nitpicks (I did not have the time to follow all the discussions, so some points may have been brough up already): 1. On compilers supporting ADL, it seems the user-supplied serialization function for non-intrusive serialization template<class Archive> void serialize(Archive & ar, gps_position & g, const unsigned int version); could also be put into the namespace of the type that it serializes. If this is correct it should be mentioned in the tutorial & reference. If not, there should be a rationale why not. 2. save/load naming: All functions that have to do with reading from an archive are named load*, all functions that have to do with writing to an archive are named save*. To me (non-native speaker) these names are generally associated with persistence only. Since the library is more general (one could use it for network communication, etc.) load/save are a bit missleading. I'd rather see read/write or other names that more clearly communicate what the functions are doing. 3. archive naming: The word archive also seems to be associated with persistence. I think serializer would much better describe the functionality. Another option I could live with is serialization_stream (I slightly disagree with the assertion in archives.html that an archive as modelled by this library is not a stream. It might not be what C++ folks traditionally understand a stream is but to me it definitely is a more general stream, i.e. a stream of objects/things/whatever). 4. Object tracking 1: Seeming contradiction I've had difficulty determining how exactly object tracking works. I believe the associated docs are slightly contradictory: - Under exceptions.html#pointer_conflict I read that the pointer_conflict exception is thrown when we save a class member through a pointer first and then "normally" (i.e. through a reference). - traits.html#tracking says "... That is[,] addresses of serialized objects are tracked if and only if an object of the same type is anywhere in the program serialized through a pointer." The second sentence seems to contradict the first one in that it does not specify that the sequence of saving is important (I assume that pointer_conflict exception is not thrown if we save normally first and then through a pointer). Moreover, I don't see how the library could possibly achieve the "anywhere in the program" bit. 5. Object tracking 2: It should not be possible to serialize pointers referencing objects of types that have track_never tracking type. special.html#objecttracking says: <quote> By definition, data types designated primitive by Implementation Level class serialization trait are never tracked. If it is desired to track a shared primitive object through a pointer (e.g. a long used as a reference count), It should be wrapped in a class/struct so that it is an identifiable type. The alternative of changing the implementation level of a long would affect all longs serialized in the whole program - probably not what one would intend. </quote> I think serializing untracked pointers almost never makes sense. If it does anyway the pointed-to object can always be saved by dereferencing the pointer. Such an error should probably be signalled with an exception. 6. Object tracking 3: One should be able to activate tracking on the fly Rarely it makes sense to save even primitive types in tracked mode (e.g. the example quoted in 6.). That's why I think setting the tracking type per type is not flexible enough. It should be possible to override the default somehow. I could imagine to save such pointers as follows: class T { ... int i; int * pI; // always points to i }; template<class Archive > void T::save(Archive &ar) const { ar << track( i ); // save the int here, keep track of the pointer ar << track( pI ); // only save a reference to the int } Since it is an error to save an untracked type by pointer even the second line needs the track( ) call. 7. I think the enum tracking_type should rather be named tracking_mode as the _type suffix suggests something else than an enum. 8. I don't understand how function instantiation as described under special.html#instantiation works. The description seems to indicate that all possible archive classes need to be included in the files of all types that are exported. Is that true? 9. The library registers all kinds of stuff with static instances. This precludes its use outside main(). This and other limitations should be stated explicitly in the rationale. ***** What is your evaluation of the implementation? ***** I did not have a look at the implementation. ***** What is your evaluation of the documentation? ***** 10. The tutorial is good. A few more links to advanced stuff could make it even easier to find your way around the library, e.g. how to split free serialization functions. 11. As other reviewers have already pointed out, the reference needs some serious work (this is not to say that Robert did a bad job, it's just not there yet). Some stuff is explained in "tutorial-language", i.e. too informal what left me wondering about the details (I can give examples if necessary). Moreover, the layout should be in C++ standard-style and include all the concepts, classes, functions and macros provided. ***** What is your evaluation of the potential usefulness of the library? ***** Very useful. ***** Did you try to use the library? With what compiler? Did you have any problems? ***** No, I did not try to use the library. ***** How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? ***** About 6 hours reading the docs and writing this review. ***** Are you knowledgeable about the problem domain? ***** Sort of. 5 years ago I tried to write a library covering a small subset of what this library does. I eventually abadoned it because of compiler problems and lack of time. ***** Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. ***** Yes. Given that the points mentioned above are resolved I vote to accept the library into boost. A big thanks to Robert for all this excellent work! Andreas