Boost logo

Boost :

From: Reece Dunn (msclrhd_at_[hidden])
Date: 2004-05-25 16:36:13


Rob Stewart wrote:
>From: "Reece Dunn" <msclrhd_at_[hidden]>
> > Rob Stewart wrote:
> > >* Should fixed_string support zero-length strings?
> >
> > You can have zero-length strings in my implementation (this is what
> > fized_string() defaults to). What you *can't* have is a zero-*capacity*
> > string, which is the same as CharT[ 0 ].
>
>I did mean zero-capacity. I didn't look at your implementation
>at this point, but I did notice that John's class declares a size
>N array, so for N == 0, his would fail to compile. Is the same
>true of yours?

yup. With mine, you'll also get an additional error, e.g.:
   fixed_string.hpp(101) : error C2039: 'value' : is not a member of
'boost::fixed_string<n>::zero_buffer_error'

Is there any reason you'd want a zero-capacity buffer?

> > >* fixed_string's at() shouldn't test the length and throw an
> > > exception since detail::basic_string_impl::at() does it and
> > > detail::basic_string_impl::operator[] calls it.
> >
> > fixed_string tests against capacity as a measure against [] usage that
>would
> > cause buffer overflow, e.g.
> > fixed_srting< 5 > str;
> > str[ 10 ] = 'a'; // ok: exception
> > vs
> > char str2[ 5 ];
> > str[ 10 ] = 'a'; // foobar
>
>Fine, but there should only need to be one test per call to
>at(). If size() can never exceed capacity, by enforcing
>invariants elsewhere, then at() only needs to check that the
>index doesn't exceed size().

But the standard specifies that at will throw if i > size(), but operator[]
does not. The i > capacity() check in the implementation is a sanity check
for operator[] and the real at function [note: I will change the name as
suggested to prevent hiding]. Therefore it is technically redundant for at,
but needed for []. I could supply an additional function (adding to those
that need to be implemented by the string provider) or add a i > capacity()
check on operator[] (deviating from the standard, but providing a sanity
check).

>If you're not going to implement get_allocator(), then it should
>be in basic_string_impl and shouldn't be among the requirements
>levied upon derivates.

ok. Do you have a way to do this?

> >From your fixed_string.hpp:
> if( l == npos ) l = StringPolicy::length( s ) + 1;

This is referring to a char_traits policy (as it operates on strings):

      template< size_t n, typename CharT = char, class StringPolicy =
std::char_traits< CharT > >
      class fixed_string;

The basic_string_impl used StringPolicy for the Derived type (now renamed to
Derived) because it was based around flex_string. Sorry for the confusion.

> > I am actually interested in both. The feedback is appreciated and I will
>use
> > it to improve the code.
>
>I'm starting to wonder if it would be good to reach some
>consensus as to which approach is better, by some definition of
>"better," so that the two of you can collaborate or one can stop
>work. I'm not certain we've reached that point, but it may be
>wise to write a common set of tests that you both can run so that
>performance can be compared (memory and speed) and binary size
>can be assessed. Given the adherence to std::basic_string's I/F,
>both classes should work with the same tests, and usability comes
>down to non-interface issues.

I agree. Although John's implementation does not yet fully implement the
basic_string interface. Mine doesn't either, but mine only lacks swap and
get_allocator (unless I have missed something).

> > To simplify my design, I could implement basic_string_impl using the
>policy
> > approach that flex_string uses. The problem with this is that it
>requires a
> > usage like:
> >
> > flex_string< fixed_string< 10, char > > str;
> >
> > so in order to get the fixed_string< n > syntax, I would need two
>classes
> > instead of just the one: this was something I was trying to move away
>from,
>
>Sorry, I don't know anything about flex_string, but what you've
>shown would only serve to make your string class more painful
>than John's (or your) current approach.

It would be easy to make it usable like it currently is:

   template< size_t n, typename CharT = char, class Traits =
std::char_traits< CharT > >
   class fixed_string: public detail::basic_string_impl<
detail::fixed_string_impl< n, CharT, Traits >, ... >
   {
      public:
         // constructors
   };

It's just that it means that you need to do this for every string
implementation. Or you could use the template typedef hack, so it would be:

   fixed_string< 10 >::type str;

but this looks odd, especially to novice users or people who don't have
experience with template metaprogramming. This was one of the motivations
for my approach as opposed to the flex_string approach.

Regards,
Reece

_________________________________________________________________
Want to block unwanted pop-ups? Download the free MSN Toolbar now!
http://toolbar.msn.co.uk/


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk