[NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

Christoph Dreis christoph.dreis at freenet.de
Thu Mar 2 07:26:06 UTC 2017


Hey Vitaly and Jonathan,

> As mentioned offline, I would move the null check right before "return this".

@Vitaly: Thanks again. Adjusted. 
@Jonathan: Thanks. Thought about that as well, but I'd probably rather go with explaining the explicit nullcheck.

See the adjusted patch below. What do you think?

===== PATCH ======
diff --git a/src/java.base/share/classes/java/lang/String.java b/src/java.base/share/classes/java/lang/String.java
--- a/src/java.base/share/classes/java/lang/String.java
+++ b/src/java.base/share/classes/java/lang/String.java
@@ -2177,11 +2177,13 @@
      */
     public String replace(CharSequence target, CharSequence replacement) {
         String tgtStr = target.toString();
-        String replStr = replacement.toString();
         int j = indexOf(tgtStr);
         if (j < 0) {
+            // Explicit nullcheck of replacement to fulfill NPE contract in early out
+            Objects.requireNonNull(replacement);
             return this;
         }
+        String replStr = replacement.toString();
         int tgtLen = tgtStr.length();
         int tgtLen1 = Math.max(tgtLen, 1);
         int thisLen = length();



More information about the core-libs-dev mailing list