XenoAmess commented on a change in pull request #560:
URL: https://github.com/apache/commons-lang/pull/560#discussion_r635324914



##########
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:
       > If `len1` is negative and if `len2` is zero then your return value 
will be whatever `len1` was, which is an incorrect result when it is not `-1`.
   
   @aherbert 
   you are correct. 
   this is a bug.
   will fix it.
   thanks.
   




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


Reply via email to