On 04/30/2013 12:43 PM, Mandy Chung wrote:
Hi Sherman,
Looks okay in general. A couple of comments:
Have you considered providing a method to map from group name to its group
index? Would that be useful?
No plan for now. It appears a bit of redundant after we have the
start/end/group(name) in place, and you can use the "name" for
the replacement already, so there is pretty much no need to get
the "index" from the name after this change.
group(String) can simply return group(getMatchedGroupIndex(name)) rather than
duplicating the implementation. Similarly for start(String) and end(String). Is the
performance overhead due to the extra check for (first < 0) and (group < 0 ||
group > groupCount()) the concern?
Yes, the "consolidation" will trigger two redundant/unnecessary (first <0) and
(group> groupCount()) checks and one layer more of method invocation. It
probably doesn't really matter though. But given it's a one-line code, I prefer
to keep it asis.
btw, start(int) and end(int) are missing the check if (group < 0).
Guess the reason why the check was missing from the original
code is that the implementation will throw the specified anyway
when access the group array next line. Though group(int) does
check the boundary and throws with a "better" error message.
Updated with the <0 check.
Nit: <tt>-1</tt> can be replaced with {@code -1}.
The reason I did not do it is that the regex uses <tt>...</tt>
and <code>...</code> everywhere, it might be better to keep
the consistency here and wait for a complete replacement
someday. But since you pointed it out, updated to use the
new style.
updated webrev:
http://cr.openjdk.java.net/~sherman/8013252/webrev/
(again, the RegExTest.java is mixed with the change for
#8013254)
Thanks!
-Sherman
Mandy
On 4/29/13 1:56 PM, Xueming Shen wrote:
Hi,
The regex named capturing group support was added into jdk7 [1].
Matcher.group(gname) is the only direct access method we added back then
to access the matched result. The proposed change here is to add a pair of
accessing/convenient method Matcher.start/end(gname) to access the start/end
offset info of the matched result, to match the corresponding start/end/group
(int index) access methods.
http://cr.openjdk.java.net/~sherman/8013252/webrev
Thanks!
-Sherman
[1] http://cr.openjdk.java.net/~sherman/6350801/webrev.02/
