[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661582#comment-14661582
]
ASF GitHub Bot commented on GROOVY-7540:
GitHub user paulk-asert opened a pull request:
https://github.com/apache/incubator-groovy/pull/80
GROOVY-7540: Add a method to StringGroovyMethods for replacing String…
… pairs supplied as a Map
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/paulk-asert/incubator-groovy groovy7540
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-groovy/pull/80.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #80
commit 96db0005e8774bc45c4111ad057ece7d09e161c3
Author: Paul King
Date: 2015-08-07T10:06:12Z
GROOVY-7540: Add a method to StringGroovyMethods for replacing String pairs
supplied as a Map
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661587#comment-14661587
]
Paul King commented on GROOVY-7540:
---
let me know if the PR looks like what you want
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661613#comment-14661613
]
Jochen Kemnade commented on GROOVY-7540:
Is the content of {{}} run as part of the test
suite? Neat!
It looks good to me. The StringBuilder initialization could be tweaked though,
it could be initialized with a definite capacity to avoid calls to
ensureCapacity later. The hard part is to know the correct size. If I'm not
mistaken, the worst case is {noformat}text.length() *
ceil(max(replacements.values()*.length()) /
min(replacements.keySet()*.length()){noformat}. And if
{noformat}replacements.keySet().find{text.indexOf(it) >= 0} == null{noformat},
we can exit early to improve the performance of things like
{noformat}'foobar'.replace([x:'y']){noformat}.
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661683#comment-14661683
]
Paul King commented on GROOVY-7540:
---
Yes, groovyTestCase is a nice feature.
There is a rather complicated guess at StringBuilder size in the Apache method
which I didn't carry over and a worse case guess could easily consume large
amounts of memory. There might be a compromise approach - perhaps something as
simple as checking if any replacement is bigger than the original and if so,
add (say) 20% otherwise just use the original size.
The early returns are well catered for - once at the beginning and then a
boolean array allows pairs to be ignored once they become irrelevant.
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661696#comment-14661696
]
Jochen Kemnade commented on GROOVY-7540:
Then I guess we still have to create a ReplaceState object even if we're not
going to replace anything. But the StringBuilder init could be moved down to
line 2888.
We could determine the correct size for the StringBuilder, but that might be
more resource-intensive than having it expand on demand. We could just go for x
* text.length() and hope that it will be enough. I'd prefer a larger factor
though, maybe even 100%.
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14662855#comment-14662855
]
Paul King commented on GROOVY-7540:
---
I tweaked to have a larger increase if longer replacements are found, but not
as large as you suggest - that could become wasteful when dealing with large
strings. Premature optimization tends to work well for the cases you have in
mind but then make other cases much worse off. So, I also provided a way to
specify your own capacity. It is leaking the implementation to some degree but
in my mind is the preferred option rather than trying to go overboard with
pre-calculations or complex heuristics. The JVM is super efficient at creating
objects and copying arrays (e.g. when expanding a StringBuilder), so we should
let it do its job most of the time unless we know our optimizations will work
well for all use cases.
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Assignee: Paul King
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14679680#comment-14679680
]
Jochen Kemnade commented on GROOVY-7540:
Looks good to me, thanks!
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Assignee: Paul King
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (GROOVY-7540) Add a method to StringGroovyMethods for replacing String pairs supplied as a Map
[
https://issues.apache.org/jira/browse/GROOVY-7540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14680961#comment-14680961
]
ASF GitHub Bot commented on GROOVY-7540:
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-groovy/pull/80
> Add a method to StringGroovyMethods for replacing String pairs supplied as a
> Map
>
>
> Key: GROOVY-7540
> URL: https://issues.apache.org/jira/browse/GROOVY-7540
> Project: Groovy
> Issue Type: Improvement
>Reporter: Jochen Kemnade
>Assignee: Paul King
>Priority: Minor
>
> It should be possible to use a map with {{collectReplacements}}, like in
> {code}
> "f006ar".collectReplacements(["0":"o", "6":"b"])
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
