Andy Little wrote:
The other surprise for me is that its possible to modify the capacity of a fixed_string.
Are you sure? You can write algorithms that don't rely on a specific capacity string. Thus, instead of: template< int n > int strlen( const fixed_string< n, char >& str ) { return str.length(); } you can write: int strlen( const fixed_string_base< char >& str ) { return str.length(); }
That means the class name fixed_string is misleading. Unless fixed means 'not broken' string implying that std::string is flawed.
The std::string interface has several flaws, such as it having too many methods (see Herb Sutter's comments on this topic) and string types that have different CharTraits not being compatible with each other. However, fixed_string does not attempt to fix these. It is short for "fixed capacity string".
Its that sense in which it doesnt promise what it delivers. I would expect a fixed_string to have a fixed length.
It has a fixed capacity, but variable length.
This fits best with the rationale of fixed_string to be a char array replacement. I suspect that is what potential users expected, but nothing like what they got so far. The solution now is to give users what they expected in the first place.
The above strlen example (and the rationale behind it) was that some users (during the initial development) wanted to be able to write operations that used fixed-capacity string such that they could redistribute the binaries only. Thus the use of the fixed_string_base. An earlier reviewer has shown me how to split these so you get the performance from using fixed_string and the ability to operate on any fixed_string< n > object without templatizing the method on n.
Concern at the lack of detail in the documentation and the way the documentation and the class itself are laid out.
I hope to improve the documentation so that users can follow it better.
Ok but there is also a lack of information. Class function signatures are documented but there is no information on what they do. As a potential user I get no sense that you as the author are keen about the class and are interested in trying to help me to get to grips with it. It looks like the documentation was sketched in and then just never worked on after that. The good thing though is that you have a potential user-base of 400 to get this right for next time.
Noted.
It would be worth going through the documentation of other (boost) libraries that you like to see how the documentation is laid out. There are also some examples of poor documentation in boost, so use those as examples of what to avoid.
The Spirit docs are very good :).
I can only fix issues that are being raised.
Whatever... You need to figure why no issues are being raised. Answer I think is that, because there is little information provided , then it is difficult to raise any points about it by telepathy so to speak.
You are thinking of a circle ;). The reviews that there have been have raised most of these points, so I will address these for the next review.
That said, I intend to move the basic_string_impl class out of the detail namespace as this is more useful in other contexts. That class is based on the flex_string class written by Andrei Alexandrescu. However, this class - like the basic_string class - is *way* too large.
I have experimented with splitting this class into several smaller ones based on the grouping in the standard documentation (iterators, capacity, etc.) so you can combine them. This way, it will be like the iterator classes. These have had some limited success and I intend on working on this after fixing the issues raised in the review.
With this change, I intend to move the fixed_string class into boost/string so that other string classes like const_string can be placed there as well.
OK. Bear in mind that you have 400 potential users who wanted a fixed_string. Its probably worth trying to find out what they thought a 'fixed_string' is and how it differs from what you provided. OTOH It sounds now like you have opted to design something completely different again, which seems a bit of a shame as you will lose many of the potential fixed_string users.
I intend to keep the behaviour of the fixed_string class (modified, according to the review feedback). This is what the unit tests are for :). It is just *how* that is implemented that will change. - Reece