On 11/5/2010 11:25 AM, Anthony Williams wrote:
- What is your evaluation of the design?
I think the design is well thought out. It cleanly separates out the basic operations for capturing, constructing, and comparing points in time from the higher-level operations of relating times to things like the Gregorian calendar. At the same time, it provides a good platform upon which these higher-level operations can be built.
- What is your evaluation of the implementation?
I have not delved into all parts of the implementation, but what I have looked at seems sound.
- What is your evaluation of the documentation?
The documentation is good. It gives a clear explanations of the basic components of the design (durations, time_points, and clocks). It also does a good job of explaining the more subtle issues of converting between different types of durations and when exact conversions are guaranteed vs. when approximation will be performed. There are small typographical or grammatical errors here and there in the documentation, but I believe others have already covered these fairly comprehensively.
- What is your evaluation of the potential usefulness of the library?
Very useful. Chrono provides the potential for an extensible and efficient way to standardize use of time across the many Boost libraries that traffic in timing, timers, and waiting (Boost.Thread, Boost.Asio, etc).
- Did you try to use the library? With what compiler? Did you have any problems?
Yes, with VC9 SP1 on Win7. I ran into some minor issues with pieces of the ratio library triggering internal compiler errors on VC9, but I was able to resolve these. I also noticed that the jam files for the regression tests reference test files that did not seem to be included in the distribution in the sandbox. Both of these should be easily fixed. I also did some testing with VC9 SP1 cross-compiling for Windows CE 5.0. I ran into some minor issues (as expected) mainly related to needing the combination of GetSystemTime and SystemTimeToFileTime calls instead of GetSystemTimeAsFileTime (which the WinCE libraries sadly do not supply). Other than this, it functioned well.
- How much effort did you put into your evaluation? A glance? A quick - reading? In-depth study?
I wouldn't say an in-depth study, but some fairly involved work. We had integrated some of the earlier versions of Boost.Chrono into our local Boost tree and had made adaptations to Boost.Thread and Boost.Asio to make use of the Chrono functionality. The Chrono-based Asio was used in some applications that do automated data collection from web pages, and Chrono-based Thread and Chrono itself were used within Win32 and WinCE applications for automated control systems. Some of the basic Chrono components were used for performance timing in tuning of application code.
- Are you knowledgeable about the problem domain?
Somewhat, but certainly not an expert.
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
Yes, I think Chrono should be accepted into Boost. Thanks, Vincente, for all your hard work in crafting this into a formal library submission, seeing through the restructuring to properly partition all the components, and filling in the documentation and testing. -Dave Deakins