From: Alexander Nasonov (alnsn_at_[hidden])
Date: 2006-01-20 07:39:55
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
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
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.
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
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-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,
./boost/fixed_string/detail/basic_string_impl.hpp: In member function
`typename Base::const_iterator boost::detail::basic_string_impl<Base,
./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?
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
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk