RFR 8013252: Regex Matcher .start and .end should be accessible by group name (original) (raw)

Xueming Shen xueming.shen at oracle.com
Tue Apr 30 20:22:54 UTC 2013

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: -1 can be replaced with {@code -1}.

The reason I did not do it is that the regex uses ... and ... 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/

More information about the core-libs-dev mailing list