-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
basic_sstring should havepush_back()
#2104
Comments
to be more compatible with basic_string<> this is also what we expect from a sequence-alike STL containers. Fixes scylladb#2104 Signed-off-by: Kefu Chai <[email protected]>
to be more compatible with basic_string<> this is also what we expect from a sequence-alike STL containers. Fixes scylladb#2104 Signed-off-by: Kefu Chai <[email protected]>
i found this when trying to use something like sstring s;
fmt::format_to(std::back_inserter(s), "{}", 1); |
This is a bad use case, since it will have quadratic behavior. Maybe it's a bad idea to implement push_back as it encourages bad performance. |
It shouldn't - according to https://en.cppreference.com/w/cpp/string/basic_string/push_back, the push_back() function has "Amortized constant" complexity, which means that pushing N characters has O(N) complexity, not O(N^2).
I realize it's harder to do the longer we wait, but I really think we should think about drop sstring and use std::string (i.e., issue #634). I don't say we should do it today, but it's something we should start considering (and should have considered many years ago). |
to be more compatible with basic_string<> this is also what we expect from a sequence-alike STL containers. Fixes scylladb#2104 Signed-off-by: Kefu Chai <[email protected]>
we need to implement
constexpr void push_back(char_type ch);
forbasic_sstring<..>
.The text was updated successfully, but these errors were encountered: