From: "Reece Dunn" <msclrhd@hotmail.com>
Rob Stewart wrote:
From: "Reece Dunn" <msclrhd@hotmail.com>
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?
My only thought was generic code that might be called upon to use a zero-capacity string. Such a string wouldn't be of much use, of course, so it is probably best to disallow it. I just wasn't sure whether yours would disallow it.
* 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).
OK. I understand your purpose now. Since the purpose of fixed_string is to provide buffer overflow protection and fixed size, and to otherwise satisfy the requirements of std::basic_string, I think it is most appropriate to add the check the one place it is needed: operator []. That avoids the added overhead for at() and puts the check precisely where it's needed. I think the right behavior is for basic_string_impl to rely on an at()-like function in fixed_string to get access to an element, and for basic_string_impl to do all of the range checking. That simplifies the job for the derived class' implementor and ensures that the tests are done correctly.
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?
Sorry, I meant to write, "then it should not be in basic_string_impl."
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 see now. It was confusing. May I suggest that StringPolicy be named "CharStringPolicy" and that fixed_string::string_policy be named "char_string_policy" or something like that? This will further distance the policy class from fixed_string itself.
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.
OK, but I'm not certain what this buys you. If it improves matters for you, then your alternative approach in which fixed_string_impl is a basic_string_impl parameter is much better. It's a one-time imposition on the developer of a derived class and has no direct impact on clients of the derived classes. The only concern I have is the (likely) slower compilation speed and worsened error messages. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;