garydgregory commented on code in PR #1318:
URL: https://github.com/apache/commons-lang/pull/1318#discussion_r1861340244
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -839,7 +839,7 @@ public static int compare(final String str1, final String
str2) {
* @since 3.5
*/
public static int compare(final String str1, final String str2, final
boolean nullIsLess) {
- if (str1 == str2) { // NOSONARLINT this intentionally uses == to allow
for both null
+ if (Objects.equals(str1, str2)) { // str1 and str2 as equal if both
are null or identical
Review Comment:
-1: You're circumventing code someone toke care to optimize and comment:
"this intentionally uses == to allow for both null"!
##########
src/main/java/org/apache/commons/lang3/BooleanUtils.java:
##########
@@ -738,7 +738,7 @@ public static Boolean toBooleanObject(final String str) {
// Optimisation provides same performance as before for interned
'true'.
// Similar performance for null, 'false', and other strings not length
2/3/4.
// 'true'/'TRUE' match 4 times slower, 'tRUE'/'True' 7 times slower.
- if (str == TRUE) {
+ if ("TRUE".equals(str)) {
Review Comment:
-1: This makes no sense to me and breaks the optimized code below in the
sense that a lot of it would now never be called. Why?
--
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]
