I vote NOT to accept it for the following reasons:
- What is your evaluation of the documentation?
The documentation says more about implementation details rather then public interface. There is only one informative sentence on fixed_string reference page: "This class provides the implementation of an n-ary fixed capacity string. It is an abstraction of a CharT[ n ] buffer that exposes a std::basic_string-style interface." All constructors and assignment operators don't have a documentation, they're only listed. All other functions are not even listed! I realise that these functions are implemented in fixed_string_base but this is not documented (I'm pretty sure it's a typo, public keyword does never come alone)
- What is your evaluation of the design?
fixed_string is derived from fixed_string_base. IIUC, the rationale is to get rid of compile-time N in fixed_string<N>. What is inappropriate, is that fixed_string copies arrays while fixed_string_base copies pointers. For example, fixed_string_base foo() { return fixed_string<10>("Return a pointer to local array"); } Truncating the string silently is also not a good way. This often leads to errors at later stage of execution thus making it diffuct to track down an origin. For example, a developer appends ".ext" in one place to make sure that all files have extention, then in another place he rseaches for '.' expecting that it never returns npos. I didn't try to compile anything because I simply can't compile anything: $ cat test.cpp #include <boost/fixed_string/fixed_string.hpp> int main() { } $ pwd /home/nasoale/src/boost/fixed_string $ ls boost libs tags test.cpp $ gcc -v Reading specs from /usr/lib/gcc-lib/i486-suse-linux/3.2.2/specs Configured with: ../configure --enable-threads=posix --prefix=/usr --with-local-prefix=/usr/local --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib --enable-languages=c,c++,f77,objc,java,ada --enable-libgcj --with-gxx-include-dir=/usr/include/g++ --with-slibdir=/lib --with-system-zlib --enable-shared --enable-__cxa_atexit i486-suse-linux Thread model: posix gcc version 3.2.2 $ g++ -I. -I ../boost_1_33_0/ test.cpp In file included from ./boost/fixed_string/fixed_string.hpp:12, from test.cpp:1: ./boost/fixed_string/detail/basic_string_impl.hpp: In member function `typename Base::const_iterator boost::detail::basic_string_impl<Base, ErrorPolicy>::begin() const': ./boost/fixed_string/detail/basic_string_impl.hpp:149: error: there are no arguments to `begin_' that depend on a template parameter, so a declaration of `begin_' must be available ./boost/fixed_string/detail/basic_string_impl.hpp:149: error: (if you use `-fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated) ... skiped a lot of error lines
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Quick reading. Sorry for all bad news for you, Reece. Es ist nicht so schlimm :))) I hope my negative review won't stop others from posting constructive comments. -- Alexander Nasonov