[ 
https://issues.apache.org/jira/browse/LANG-1572?focusedWorklogId=597393&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-597393
 ]

ASF GitHub Bot logged work on LANG-1572:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 16/May/21 15:35
            Start Date: 16/May/21 15:35
    Worklog Time Spent: 10m 
      Work Description: aherbert commented on a change in pull request #560:
URL: https://github.com/apache/commons-lang/pull/560#discussion_r633049410



##########
File path: src/main/java/org/apache/commons/lang3/CharSequenceUtils.java
##########
@@ -133,24 +133,87 @@ static int indexOf(final CharSequence cs, final int 
searchChar, int start) {
      * @param start the start index
      * @return the index where the search sequence was found
      */
-    static int indexOf(final CharSequence cs, final CharSequence searchChar, 
final int start) {
-        if (cs instanceof String) {
-            return ((String) cs).indexOf(searchChar.toString(), start);
-        } else if (cs instanceof StringBuilder) {
-            return ((StringBuilder) cs).indexOf(searchChar.toString(), start);
-        } else if (cs instanceof StringBuffer) {
-            return ((StringBuffer) cs).indexOf(searchChar.toString(), start);
-        }
-        return cs.toString().indexOf(searchChar.toString(), start);
-//        if (cs instanceof String && searchChar instanceof String) {
-//            // TODO: Do we assume searchChar is usually relatively small;
-//            //       If so then calling toString() on it is better than 
reverting to
-//            //       the green implementation in the else block
-//            return ((String) cs).indexOf((String) searchChar, start);
-//        } else {
-//            // TODO: Implement rather than convert to String
-//            return cs.toString().indexOf(searchChar.toString(), start);
-//        }
+    static int indexOf(final CharSequence cs, final CharSequence searchChar, 
int start) {
+        // if searchChar is a String, and cs is some specific kind of 
AbstractString,
+        //   which both have a indexOf function and widely used by normal java 
codes,
+        // then we just invoke their indexOf function.
+        if (searchChar instanceof String) {
+            if (cs instanceof String) {
+                return ((String) cs).indexOf((String) searchChar, start);
+            } else if (cs instanceof StringBuilder) {
+                return ((StringBuilder) cs).indexOf((String) searchChar, 
start);
+            } else if (cs instanceof StringBuffer) {
+                return ((StringBuffer) cs).indexOf((String) searchChar, start);
+            }
+        }
+
+        int len1 = cs.length();
+        int len2 = searchChar.length();
+
+        // successful-result >= 0
+        if (start < 0) {
+            start = 0;
+        }
+
+
+        // successful-result <= len1
+        if (start > len1) {
+            start = len1;
+        }
+
+        // if len2 == 0 return directly.
+        if (len2 == 0) {
+            return start;
+        }
+
+        // limit means the largest possible value of successful-result
+        final int limit = len1 - len2;
+
+        // if start > limit, then have no enough length for a search,
+        //   thus cannot find a index, thus fail.
+        // if len2 < 0, then it is illegal for this function.
+        // if limit < 0, then it means len2 > len1, thus fail.
+        if (start > limit || len2 < 0 || limit < 0) {
+            return -1;
+        }
+
+        // notice that when we enter here, we make sure:
+        // start in [0, limit]
+        // limit in [0, len1-1]
+        // len2 in [1, len1]
+
+        // if len2 is small enough, and cs is some specific kind of 
AbstractString,
+        //   which both have a indexOf function and widely used by normal java 
codes,
+        // then we just invoke their indexOf function.
+        if (len2 <= TO_STRING_LIMIT) {
+            if (cs instanceof String) {
+                return ((String) cs).indexOf(searchChar.toString(), start);
+            } else if (cs instanceof StringBuilder) {
+                return ((StringBuilder) cs).indexOf(searchChar.toString(), 
start);
+            } else if (cs instanceof StringBuffer) {
+                return ((StringBuffer) cs).indexOf(searchChar.toString(), 
start);
+            }
+        }
+
+        char char0 = searchChar.charAt(0);
+
+        int i = start;
+        while (true) {

Review comment:
       No need for the ugly double while loop:
   ```java
   for (int i = start; i <= limit; i++) {
       if (cs.charAt(i) == char0 && checkLaterThan1(cs, searchChar, len2, i)) {
           return i;
       }
   }
   return -1;
   ```

##########
File path: src/main/java/org/apache/commons/lang3/CharSequenceUtils.java
##########
@@ -133,24 +133,87 @@ static int indexOf(final CharSequence cs, final int 
searchChar, int start) {
      * @param start the start index
      * @return the index where the search sequence was found
      */
-    static int indexOf(final CharSequence cs, final CharSequence searchChar, 
final int start) {
-        if (cs instanceof String) {
-            return ((String) cs).indexOf(searchChar.toString(), start);
-        } else if (cs instanceof StringBuilder) {
-            return ((StringBuilder) cs).indexOf(searchChar.toString(), start);
-        } else if (cs instanceof StringBuffer) {
-            return ((StringBuffer) cs).indexOf(searchChar.toString(), start);
-        }
-        return cs.toString().indexOf(searchChar.toString(), start);
-//        if (cs instanceof String && searchChar instanceof String) {
-//            // TODO: Do we assume searchChar is usually relatively small;
-//            //       If so then calling toString() on it is better than 
reverting to
-//            //       the green implementation in the else block
-//            return ((String) cs).indexOf((String) searchChar, start);
-//        } else {
-//            // TODO: Implement rather than convert to String
-//            return cs.toString().indexOf(searchChar.toString(), start);
-//        }
+    static int indexOf(final CharSequence cs, final CharSequence searchChar, 
int start) {
+        // if searchChar is a String, and cs is some specific kind of 
AbstractString,
+        //   which both have a indexOf function and widely used by normal java 
codes,
+        // then we just invoke their indexOf function.
+        if (searchChar instanceof String) {
+            if (cs instanceof String) {
+                return ((String) cs).indexOf((String) searchChar, start);
+            } else if (cs instanceof StringBuilder) {
+                return ((StringBuilder) cs).indexOf((String) searchChar, 
start);
+            } else if (cs instanceof StringBuffer) {
+                return ((StringBuffer) cs).indexOf((String) searchChar, start);
+            }
+        }
+
+        int len1 = cs.length();
+        int len2 = searchChar.length();
+
+        // successful-result >= 0
+        if (start < 0) {
+            start = 0;
+        }

Review comment:
       } else if (start > len1) {




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 597393)
    Time Spent: 1h 40m  (was: 1.5h)

> implement TODO in CharSequenceUtils.indexOf : remake it.
> --------------------------------------------------------
>
>                 Key: LANG-1572
>                 URL: https://issues.apache.org/jira/browse/LANG-1572
>             Project: Commons Lang
>          Issue Type: Sub-task
>            Reporter: Jin Xu
>            Priority: Major
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> [https://github.com/apache/commons-lang/pull/560]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to