Reputation: 2893
What is wasted in the example from the Cpp Core Guidelines?
P.9: Don't waste time or space
[...]
void lower(zstring s) { for (int i = 0; i < strlen(s); ++i) s[i] = tolower(s[i]); }
Yes, this is an example from production code. We leave it to the reader to figure out what's wasted.
from https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rp-waste
Upvotes: 11
Views: 3108
Reputation: 11379
Unless they have any very unintuitive semantics in the zstring
class, the function in it's current form is a complete waste of both time and space, as its "result" can't be used after the function - it is passed in as value, and isn't returned.
So to avoid wasting the time to uselessly compute the lowercase which can't be used, and the space in copying the passed parameter, I would pass by reference!
Upvotes: 0
Reputation: 63124
As other aswers have already stated, strlen(s)
is called multiple times because it is in the condition, implying that it should be cached and reused instead.
But strlen(s)
does not actually need to be called at all ! s
is (or is implicitly convertible to) a nul-terminated char
array, since that's what strlen
expects. So we can just use this very property for our own loop.
void lower(zstring s) {
for (char *p = s; *p; ++p)
*p = std::tolower((unsigned char)*p);
}
Upvotes: 5
Reputation: 44838
A lot of time is wasted and a segmentation fault may occur as the author of the code's increasing s
, not i
in the loop:
for (int i = 0; i < strlen(s); ++s)
//right here ^^^^
Upvotes: 7
Reputation: 119164
strlen
is called every time the loop condition is checked, and takes O(n) time per call, so the total time for the loop is O(n^2).
Upvotes: 12