commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Neidhart (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LANG-1077) [PATCH] StringUtils.ordinalIndexOf("aaaaaa", "aa", 2) != 3 in StringUtils
Date Fri, 11 Dec 2015 19:46:46 GMT

    [ 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)

Mime
View raw message