[ 
https://issues.apache.org/jira/browse/LANG-1077?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15052734#comment-15052734
 ] 

Sebb commented on LANG-1077:
----------------------------

On further reflection, I think the original bug report was wrong, and the 
original code is correct.

The original code steps through the search target one char at a time, so I 
think the behaviour should be:

StringUtils.ordinalIndexOf("aaaaaa", "aa", 1) => 0 // matches "aa"
StringUtils.ordinalIndexOf("aaaaaa", "aa", 2) => 1 // matches ".aa"

However if overlapping matches are not allowed, I would expect:

StringUtils.ordinalIndexOf("aaaaaa", "aa", 2) => 2 // matches "..aa"

I have no idea why it was ever expected that the result should be "3", unless 
it was thought that the index started at 1.

Perhaps the OP expected overlapping matches to be disallowed.
However there is  no indication that this was ever intended.

The fix needs to be reverted, and the Javadoc updated to make it clear that 
overlapping matches are allowed.

> [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
>             Fix For: 3.4
>
>         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)

Reply via email to