[
https://issues.apache.org/jira/browse/LANG-1077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15053437#comment-15053437
]
Thomas Neidhart commented on LANG-1077:
---------------------------------------
This code exists since a long time and looking at the implementation it is
clear what it does and what its intention is: repeatedly execute indexOf with a
start index of +1 compared to the last match. There are even suggestions on
stackoverflow to use this method for exactly that use-case.
What you are facing is a corner-case that was not described in the javadoc and
thus it might create confusion for a user. So the right thing to do is what
sebb did: revert the fix to the previous, working version and update the
javadoc to document the corner case. Contrary to your opinion that this might
introduce bugs, it is quite the opposite, your fix will introduce bugs as
people relied on the previous version for a long time already.
If there is a need for a method that does not handle overlaps, then this should
be a new feature request. The method could have an additional flag to indicate
how it handles overlaps.
> [PATCH] StringUtils.ordinalIndexOf("aaaaaa", "aa", 2) != 3 in StringUtils
> --------------------------------------------------------------------------
>
> Key: LANG-1077
> URL: https://issues.apache.org/jira/browse/LANG-1077
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.*
> Affects Versions: 3.3.2
> Reporter: haiyang li
> Labels: patch
> Attachments: LANG-1077.patch
>
>
> {code:title= org.apache.commons.lang3.StringUtils.java|borderStyle=solid}
> int found = 0;
> int index = lastIndex ? str.length() : INDEX_NOT_FOUND;
> do {
> if (lastIndex) {
> index = CharSequenceUtils.lastIndexOf(str, searchStr, index -
> 1);
> } else {
> index = CharSequenceUtils.indexOf(str, searchStr, index + 1);
> }
> if (index < 0) {
> return index;
> }
> found++;
> } while (found < ordinal);
> {code}
> Should it be:
> {code:title= org.apache.commons.lang3.StringUtils.java|borderStyle=solid}
> private static int ordinalIndexOf(final CharSequence str, final
> CharSequence searchStr, final int ordinal, final boolean lastIndex) {
> // if (str == null || searchStr == null || ordinal <= 0) {
> // return INDEX_NOT_FOUND;
> // }
> // if (searchStr.length() == 0) {
> // return lastIndex ? str.length() : 0;
> // }
> // int found = 0;
> // int index = lastIndex ? str.length() : INDEX_NOT_FOUND;
> // do {
> // if (lastIndex) {
> // index = CharSequenceUtils.lastIndexOf(str,
> searchStr, index - 1);
> // } else {
> // index = CharSequenceUtils.indexOf(str, searchStr,
> index + 1);
> // }
> // if (index < 0) {
> // return index;
> // }
> // found++;
> // } while (found < ordinal);
> // return index;
> if (str == null || searchStr == null || ordinal <= 0) {
> return INDEX_NOT_FOUND;
> }
> if (searchStr.length() == 0) {
> return lastIndex ? str.length() : 0;
> }
> final int searchStrLen = searchStr.length();
> int index = lastIndex ? str.length() : 0;
> for (int found = 0; index >= 0;) {
> if (lastIndex) {
> index = CharSequenceUtils.lastIndexOf(str, searchStr, index);
> } else {
> index = CharSequenceUtils.indexOf(str, searchStr, index);
> }
> if (index < 0) {
> return INDEX_NOT_FOUND;
> }
> if (++found >= ordinal) {
> break;
> }
> index = lastIndex ? index - searchStrLen : index + searchStrLen;
> }
> return index;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
