hamster on wheels
hamster on wheels

Reputation: 2893

What is wasted in this example from the Cpp Core Guidelines?

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

Answers (5)

codeling
codeling

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

Quentin
Quentin

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

ForceBru
ForceBru

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

Brian Bi
Brian Bi

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

user31264
user31264

Reputation: 6727

strlen is calculated at every iteration of the loop.

Upvotes: 29

Related Questions