patterncppMinor
Repeating a std::string n times
Viewed 0 times
stdtimesrepeatingstring
Problem
I recently posted an answer to this SO question as I did not think any of the current answers were satisfactory (most don't even answer the question!). I was wondering if anyone could offer any improvements on my solution to this problem? I'm most interested in performance here. Also note this algorithm does not require any additional copies of the input string (if passed an r-value), ideally I'd like to keep this property.
Here is an example:
#include
#include // std::size_t
#include // std::floor, std::log2, std::pow
#include // std::cbegin, std::cend, std::next
std::string repeatstring(std::string str, std::size_t n)
{
if (n == 0) {
str.clear();
str.shrink_to_fit();
return str;
}
if (n == 1 || str.empty()) return str;
const auto repeat_size = str.size();
if (repeat_size == 1) {
str.append(n - 1, str.front());
return str;
}
str.reserve(repeat_size * n);
const auto m = static_cast(std::floor(std::log2(n)));
for (auto i = m; i > 0; --i) str += str;
n -= static_cast(std::pow(2, m));
str.append(std::cbegin(str), std::next(std::cbegin(str), n * repeat_size));
return str;
}Here is an example:
#include
#include
int main()
{
auto start = std::chrono::system_clock::now();
auto str = repeatstring("helloworld", 190238011);
auto end = std::chrono::system_clock::now();
//std::cout (end - start);
std::cout << "time: " << duration.count() << "ms" << std::endl;
return 0;
}Solution
Input as reference-to-const
Change your signature to:
First, this avoids an unnecessary copy when users don't pass their strings in by rvalue (which seems more typical?). Secondly, this allows for copy elision on the returned object, as you can't have copy elision from a function parameter. So if I called
Also, note the name change. Either use
The new signature makes some of the other code easier.
This is a weird variable at best. Once you make
Powers of 2
Now
with:
And:
is a really complicated way of writing what now becomes:
Change your signature to:
std::string repeat_string(std::string const& str, std::size_t N);First, this avoids an unnecessary copy when users don't pass their strings in by rvalue (which seems more typical?). Secondly, this allows for copy elision on the returned object, as you can't have copy elision from a function parameter. So if I called
repeat_string with an lvalue, I just saved two copies.Also, note the name change. Either use
camelCase or snake_case or function names, don't just jamlotsofwordstogether. It's much harder to read.The new signature makes some of the other code easier.
if (n == 0) {
return {};
}
...
if (str.size() == 1) {
return std::string(n str[0]);
}repeat_sizeThis is a weird variable at best. Once you make
str a reference to const, you can just access str.size() which makes much more sense. repeat_size isn't really the size of the repeat - that's what n is...Powers of 2
std::log2 and std::pow aren't cheap, and you need neither. For the former, you can just multiply by two until you're done:std::string result = str;
result.reserve(str.size() * n);
std::size_t m = 2;
for (; m <= n; m *= 2)
{
result += result;
}Now
m is the first power of 2 larger than n. So you can replace n -= static_cast(std::pow(2, m));with:
n -= m/2;And:
str.append(std::cbegin(str), std::next(std::cbegin(str), n * repeat_size));is a really complicated way of writing what now becomes:
result.append(result.c_str(), n * str.size());Code Snippets
std::string repeat_string(std::string const& str, std::size_t N);if (n == 0) {
return {};
}
...
if (str.size() == 1) {
return std::string(n str[0]);
}std::string result = str;
result.reserve(str.size() * n);
std::size_t m = 2;
for (; m <= n; m *= 2)
{
result += result;
}n -= static_cast<decltype(n)>(std::pow(2, m));str.append(std::cbegin(str), std::next(std::cbegin(str), n * repeat_size));Context
StackExchange Code Review Q#114295, answer score: 7
Revisions (0)
No revisions yet.