garydgregory commented on code in PR #1327:
URL: https://github.com/apache/commons-lang/pull/1327#discussion_r1869415978
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -2829,41 +2835,23 @@ public static int indexOfAny(final CharSequence cs,
final String searchChars) {
* </pre>
*
- * @param cs the CharSequence to check, may be null
+ * @param srcChars the CharSequence to check, may be null
Review Comment:
Don't rename parameters in methods, it makes the PR noisy and makes it
harder to review.
##########
src/test/java/org/apache/commons/lang3/Supplementary.java:
##########
@@ -25,25 +25,24 @@
public class Supplementary {
/**
- * Supplementary character U+20000 See
https://www.oracle.com/technical-resources/articles/javase/supplementary.html
+ * Incomplete supplementary character U+20000, low surrogate only. See
+ *
https://www.oracle.com/technical-resources/articles/javase/supplementary.html
*/
- static final String CharU20000 = "\uD840\uDC00";
+ static final String CharU20000SupplCharLow = "\uDC00";
Review Comment:
These changes duplicate your changes in your other PR. Rebase on git master
and don't make these changes here.
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -2878,27 +2866,30 @@ public static int indexOfAnyBut(final CharSequence cs,
final char... searchChars
* StringUtils.indexOfAnyBut("aba", "ab") = -1
* </pre>
*
- * @param seq the CharSequence to check, may be null
+ * @param srcChars the CharSequence to check, may be null
* @param searchChars the chars to search for, may be null
* @return the index of any of the chars, -1 if no match or null input
* @since 2.0
* @since 3.0 Changed signature from indexOfAnyBut(String, String) to
indexOfAnyBut(CharSequence, CharSequence)
*/
- public static int indexOfAnyBut(final CharSequence seq, final CharSequence
searchChars) {
- if (isEmpty(seq) || isEmpty(searchChars)) {
+ public static int indexOfAnyBut(final CharSequence srcChars, final
CharSequence searchChars) {
+ if (srcChars == null || searchChars == null || srcChars.length() == 0
|| searchChars.length() == 0) {
Review Comment:
Not sure why you inlined these calls. Keep the PR to only the required
changes to fix a bug.
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -2878,27 +2866,30 @@ public static int indexOfAnyBut(final CharSequence cs,
final char... searchChars
* StringUtils.indexOfAnyBut("aba", "ab") = -1
* </pre>
*
- * @param seq the CharSequence to check, may be null
+ * @param srcChars the CharSequence to check, may be null
* @param searchChars the chars to search for, may be null
* @return the index of any of the chars, -1 if no match or null input
* @since 2.0
* @since 3.0 Changed signature from indexOfAnyBut(String, String) to
indexOfAnyBut(CharSequence, CharSequence)
*/
- public static int indexOfAnyBut(final CharSequence seq, final CharSequence
searchChars) {
- if (isEmpty(seq) || isEmpty(searchChars)) {
+ public static int indexOfAnyBut(final CharSequence srcChars, final
CharSequence searchChars) {
+ if (srcChars == null || searchChars == null || srcChars.length() == 0
|| searchChars.length() == 0) {
return INDEX_NOT_FOUND;
}
- final int strLen = seq.length();
- for (int i = 0; i < strLen; i++) {
- final char ch = seq.charAt(i);
- final boolean chFound = CharSequenceUtils.indexOf(searchChars, ch,
0) >= 0;
- if (i + 1 < strLen && Character.isHighSurrogate(ch)) {
- final char ch2 = seq.charAt(i + 1);
- if (chFound && CharSequenceUtils.indexOf(searchChars, ch2, 0)
< 0) {
- return i;
- }
- } else if (!chFound) {
- return i;
+ final Set<Integer> srcSetCodePoints =
srcChars.codePoints().boxed().collect(Collectors.toSet()); // >=10:
Collectors::toUnmodifiableSet
Review Comment:
What does comments like >=10, 11, ... mean here? Can you be more explicit?
Are these magic numbers?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
