paulk-asert commented on code in PR #2254: URL: https://github.com/apache/groovy/pull/2254#discussion_r2153724057
########## src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java: ########## @@ -1547,6 +1548,146 @@ private static List<String> getGroups(final Matcher matcher) { return list; } + // stateful matcher API is not very extensible; care should be taken if using the matcher directly as it will potentially be mutated + // this mostly provides an efficient list delegate but will less efficiently recalculates matches for named groups + static class GroupList implements List<String> { + private final Matcher matcher; + private final int count; + private final List<String> delegate; + + public GroupList(Matcher matcher, int count) { + this.matcher = matcher; + this.count = count; + this.delegate = getGroups(matcher); + } + + @Override + public int size() { + return delegate.size(); + } + + @Override + public boolean isEmpty() { + return delegate.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return delegate.contains(o); + } + + @Override + public Iterator<String> iterator() { + return delegate.iterator(); + } + + @Override + public Object[] toArray() { + return delegate.toArray(); + } + + @Override + public boolean add(String s) { + throw new UnsupportedOperationException("Can't add to a Matcher result"); + } + + @Override + public boolean remove(Object o) { + throw new UnsupportedOperationException("Can't remove a Matcher result"); + } + + @Override + public boolean addAll(Collection c) { + throw new UnsupportedOperationException("Can't addAll to a Matcher result"); + } + + @Override + public boolean addAll(int index, Collection c) { + throw new UnsupportedOperationException("Can't addAll to a Matcher result"); + } + + @Override + public void clear() { + throw new UnsupportedOperationException("Can't clear a Matcher result"); + } + + @Override + public String get(int index) { + return delegate.get(index); + } + + public String get(String name) { + matcher.reset(); + for (int i = 0; i <= count; i++) { Review Comment: It all looks fine to me. You have to call find() at least once before accessing results and we have index 0 for that case. If we changed it from `count++` to `++count`, we'd need to follow the advice given. ``` def issueRegex = /(?<project>\w*)-(?<number>\d+)/ def m = 'GROOVY-11701 precedes GROOVY-11702 and follows GROOVY-11700' =~ issueRegex assert m[0][0] == 'GROOVY-11701' assert m[0][1] == 'GROOVY' assert m[0][2] == '11701' assert m[0]['project'] == 'GROOVY' assert m[0].number == '11701' assert m[1][0] == 'GROOVY-11702' assert m[1][1] == 'GROOVY' assert m[1][2] == '11702' assert m[1]['project'] == 'GROOVY' assert m[1].number == '11702' assert m[2][0] == 'GROOVY-11700' assert m[2][1] == 'GROOVY' assert m[2][2] == '11700' assert m[2]['project'] == 'GROOVY' assert m[2].number == '11700' ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@groovy.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org