On 27/11/2019 06:33, Vinnie Falco wrote:
I would much prefer static_string
Same here... I was foolish to rename it.
Neither one is a great choice, as both "static" and "fixed" can be synonyms of "immutable", which this is not. "fixed_capacity_string" is the most accurate name -- while you might think that's too long, I think it's better to be properly descriptive and allow consumers to typedef it to something easier to type if they wish to.
Regarding your deviations from std::string: operator+ has been omitted because you consider it's not possible to determine a reasonable return type; isn't it just size N+M ? Do you reject that just because it could end up rather large after a sequence of additions? I'd agree if it were N*M but not really for N+M. Have I missed something?
Yes. It is too easy for someone to change existing code that does a+b+c+d where the operands are strings. Users could the get surprising behavior of tens or hundreds of kilobytes of stack consumption. There are other ways to implement operator+ but we prefer to the omit interfaces that can have surprising behavior, or that have non-obvious results (such as truncating).
The most logical capacity for the result of operator+ would probably be std::max(a.capacity(), b.capacity()). Bear in mind that we are assuming that both actual string values are significantly shorter than their capacities throughout most of their lifetime. (This is a reasonably safe assumption especially when you *don't* have an operator"" that produces exactly-right-sized strings.) In an ideal world, you'd be able to factor in the current lengths, but of course that isn't available at compile time so can't be used to determine the type. Having said that, operator+ is still going to cause an allocation, so it does still make sense to omit it from the interface for that reason, so that people think about it more (usually by switching to zero-allocation append or +=).
You've chosen to return a string_view from substr(). I think that's a bit dangerous; code that does "auto a = s.substr(...)" could end up with a dangling view when s is a fixed string; this makes it more difficult than it should be to migrate from one string type to another, or to write generic code.
Again we have to refer to the purpose of the container. People are using this for performance, the very last thing they want from substr() is to receive a copy. Users can opt-in to making a copy if they want, by constructing a new static_string from the string view returned by substr. If we go with your suggestion, no one can opt out of copies.
Perhaps as a compromise (due to the difference from std::string interface anyway) this method should be named substr_view? This would then alert someone converting from one to the other to look at each call site and verify that it isn't going to produce a dangling view.
My initial thought was that static_string should be implemented on top of boost::container::static_vector<char>. My rationale...
No normal user is going at static_string and think "I would use this if it required Boost.Container." Fewer dependencies is better. And, this library is designed to work without the rest of Boost (the "standalone" mode). I want people to be able to do "vcpkg install static_string" and then be ready to boogie. They will still have to type `boost::` to access it.
That just means that standalone mode would need to depend on a standalone implementation of static_vector, which vcpkg would pull in as well. Or you should give up on a standalone mode and have your vcpkg depend on Boost. Dependencies can be annoying, but constantly reinventing existing vocabulary types is also annoying. Waiting until they get adopted by the standard before using them in other libraries is a bit silly.