11 Oct
2010
11 Oct
'10
8:44 p.m.
Tan, Tom (Shanghai) wrote: > > ----- Original Message ----- > From: "Anthony Williams" <anthony.ajw@gmail.com> > Subject: [boost] Reminder: [Review] Formal Review of Proposed > Boost.RatioLibrary IS UNDERWAY >>> Here are some questions you might want to answer in your review: >>> >>> - What is your evaluation of the implementation? > - Code is clear and easy to read and understood. > - BOOST_INTMAX_C is the only macro that's defined in ratio.hpp but not > staring with BOOST_RATIO_. As the name suggests, it'll be better 1) if > placed in boost/cstdint.hpp, or 2)be named as BOOST_RATIO_INTMAX_C > indicating that it's used for boost.ratio only, I personally prefer > option 1). > I'll see if we can add it to cstdint.hpp. Waiting for that I will rename the macro. > - I would expect the comments are better cleaned up and the style > unified. Right now, //, //-, //~, /**/ are all used, which seems > confusing. I personally like the style adopted in boost.asio. > I don't see any major problem with that, but I'll try to be more uniform. >>> - What is your evaluation of the documentation? > - In the tutorial section, it'll be great if more examples are appended > so that one example for each supported arithmetic and comparison > operations, considering boost.ratio is a small one, doing so won't > explode the tutorial section but gives a more complete picture of the > lib. > You are right, these are no documented on the tutorial. Even if its use is simple, I will try to find an example that show the utility. > (Can anyone please let me know whether ratio is short for rational or a > complete word itself? It seems either is OK, though I still wonder) > Ratio comes from Latin and means proportion (the word exists in some language like English or French as such). Rational is a number. >> - Did you try to use the library? With what compiler? Did you have any problems? Yes, indirectly through using boost.chrono, which is based on boost.ratio. VC10 on Windows 7, GCC4.4 w/ mingwin on widows 7. GCC4.4 on Ubuntu 10.4 No problem with the latest version from boost sandbox. >> - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I did have some in-depth reading of previous versions of the code & documentation while I was started using boost.chrono, though not line by line. I also did a deep reading of C++ Standards Committee's current Working Paper (n2661in order to understand the why and how of ratio class then. While doing this review, I had a thorough reading of the code and documentation, which costs about half a day (including expressing my comments in English). >> - Do you think the library should be accepted as a Boost library? Yes. Thanks Tom, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/Re-Review-Formal-Review-of-Proposed-Boost-RatioLibrary-IS-UNDERWAY-tp2983933p2990595.html Sent from the Boost - Dev mailing list archive at Nabble.com.