Re: [boost] Re: static sized strings
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?
* 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/
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;
Since space for a trailing null is required, the minimum "capacity" is currently 1. The maximum value for "size()" is then "capacity()-1". That seems a little wierd. Should the trailing null be counted in "capacity?" John Nagle Animats Rob Stewart wrote:
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.
From: John Nagle <nagle@animats.com>
Since space for a trailing null is required, the minimum "capacity" is currently 1.
This means your class supports strings that can never contain any data. Is that something that should be permissible?
The maximum value for "size()" is then "capacity()-1". That seems a little wierd. Should the trailing null be counted in "capacity?"
No, capacity() should return the maximum value size() can attain. Think about the typical approach to allocating a C string: size_t const MAX_LENGTH(10); char buffer[MAX_LENGTH + 1]; This permits using MAX_LENGTH in comparisons with strlen(buffer). I think capacity() is analogous to MAX_LENGTH. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Rob Stewart wrote:
From: John Nagle <nagle@animats.com>
Since space for a trailing null is required, the minimum "capacity" is currently 1.
This means your class supports strings that can never contain any data. Is that something that should be permissible?
That's actually useful. Consder the
The maximum value for "size()" is then "capacity()-1". That seems a little wierd. Should the trailing null be counted in "capacity?"
No, capacity() should return the maximum value size() can attain. Think about the typical approach to allocating a C string:
size_t const MAX_LENGTH(10); char buffer[MAX_LENGTH + 1];
This permits using MAX_LENGTH in comparisons with strlen(buffer). I think capacity() is analogous to MAX_LENGTH.
From: John Nagle <nagle@animats.com>
Rob Stewart wrote:
From: John Nagle <nagle@animats.com>
Since space for a trailing null is required, the minimum "capacity" is currently 1.
This means your class supports strings that can never contain any data. Is that something that should be permissible?
That's actually useful. Consder the ^^^^^^^^^^^ I assume you send your message before completing your thought.
The maximum value for "size()" is then "capacity()-1". That seems a little wierd. Should the trailing null be counted in "capacity?"
No, capacity() should return the maximum value size() can attain. Think about the typical approach to allocating a C string:
size_t const MAX_LENGTH(10); char buffer[MAX_LENGTH + 1];
This permits using MAX_LENGTH in comparisons with strlen(buffer). I think capacity() is analogous to MAX_LENGTH.
-- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
participants (3)
-
John Nagle -
Reece Dunn -
Rob Stewart