Added Multi-Replacement proc for strings #6193
Conversation
…he length of the original string
To prevent endless replacement recursion, replacements cannot overlap. Earlier replacements in the argument list are preferred over later overlapping replacements.
|
I dont think word |
|
@cheatfate optimized in regards to memory allocations... calling replace multiple times instead is worse! |
|
@couven92 if you think |
|
@cheatfate even if I preallocate like I do in the beginning of the proc? |
|
You allocate one more copy of string in result, what if replacement values much more in size then replacement markers? |
|
Of course if you replace 'a' with 'b' there will be no additional allocations, but what if you replace 'a' with 'aaaaaaaaaaaaa'? |
|
yeah, I know... but nothing much we can do about that... But I agree that the documentation comment can be misleading... Any suggestions? If you have a more optimized version in mind, I'd be happy to hear about it as well :) |
lib/pure/strutils.nim
Outdated
| ## The order of the replacements does matter. Earlier replacements are preferred over later | ||
| ## replacements in the argument list. | ||
| result = newStringOfCap(s.len) | ||
| var charsRead = 0 |
lib/pure/strutils.nim
Outdated
| result = newStringOfCap(s.len) | ||
| var charsRead = 0 | ||
| while charsRead < s.len: | ||
| var noReplacement = true |
There was a problem hiding this comment.
You can use a 'block label' + 'break label' here instead of the bool 'noReplacement' flag.
|
There still many ways to improve your code, for example Then you can call it |
|
Okay @cheatfate I have changed @Araq thanks for the review, have done as you asked. YES! Another case where I can use the |
|
Optimize it further: var fastChk: set[char] = {}
for r in replacements: incl(fastChk, r[0][0])
...
# assume most chars are not candidates for any replacement operation:
if s[i] in fastChk:
...
|
Assume most chars in s are not candidates for any replacement operation
|
@Araq like this? |
lib/pure/strutils.nim
Outdated
| var i = 0 | ||
| while i < s.len: | ||
| block sIteration: | ||
| var fastChk: set[char] = {} |
There was a problem hiding this comment.
You need to move 'fastChk' construction out of the 'while' loop.
lib/pure/strutils.nim
Outdated
| while i < s.len: | ||
| block sIteration: | ||
| var fastChk: set[char] = {} | ||
| for tup in replacements: fastChk.incl(tup[0][0]) # Include first character of all replcaments |
There was a problem hiding this comment.
This includes this 'for' loop too, of course.
There was a problem hiding this comment.
Ah! right of course! :P
My brain has gone on holiday it seems!
No description provided.