Cached result size longer than expected

[tbusser]

While studying the code I came across a use case which displayed behaviour I wasn't expecting. I was wondering if it was by design or something that went unnoticed.

Steps to reproduce

• Repeat the string 'a', 3 times
• Repeat the string 'a', 5 times

The unexpected behaviour The result outputted by repeat-string is correct for both calls. The unexpected behaviour is in the value of res after the second call to repeat-string. I was expecting this to be aaaaa (5 times) but its value was aaaaaaaa (8 times).

Source of the unexpected behaviour The reason the value of res was different than expected is because the method doesn't take into account the current length of the cached string when it starts the while loop. It just starts with num = 5 and performs the while loop until the size of res exceeds the length of the string to return.

Possible solution A possible solution would be to check if the length of the result string is bigger than the current size of the cached string. When this is the case the value of num should be adjust so it only covers the difference between the length of the current cached string and the length of the result string.

if (res.length < max) {
    num = (max - res.length) / str.length;
}

I've benchmarked repeat-string with this change in combination with pad-left and the performance hit is negligible. It could however help to keep the memory footprint smaller by making sure the cached result is never longer than the longest requested string.

I am really not sure if it would make any difference at all in the real world, it was just something that sparked my interest.

[jonschlinkert]

To clarify, is the actual result incorrect, or just the cached value of res before it's returned and sliced by substr?

[tbusser]

The actual result is fine, exactly as expected. It is only the internal value of res which was different from what I was expecting.

[jonschlinkert]

I think it's fine, mainly because we don't know what the next requested length will be - or if there will be one, so we're incurring a penalty on the current function call (even if it is negligible).

maybe we could do something like this:

res = res.substr(0, max);
return res;

Which shouldn't incur any penalty at all, and at least sets the cache to the returned value

[jonschlinkert]

Btw, you should see the difference caching makes with compiling regexes. Like if you have a function that does this:

var regex = new RegExp('foo' + something + 'bar')`;

Then use an object to cache something with the compiled regex.

[jonschlinkert]

closing since the pr was merged