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

[Vitaly Davidovich]

As mentioned offline, I would move the null check right before "return
this".
On Wed, Mar 1, 2017 at 6:50 PM Christoph Dreis <christoph.dreis at freenet.de>
wrote:

> Hey Vitaly,
>
> > Seems like a good idea.  You probably want to null check 'replacement'
> before the bail out as the method is specified as throwing NPE in that case.
>
> Thanks for your feedback. See the adjusted patch below.
>
> ===== 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
> @@ -2176,12 +2176,13 @@
>       * @since 1.5
>       */
>      public String replace(CharSequence target, CharSequence replacement) {
> +        Objects.requireNonNull(replacement);
>          String tgtStr = target.toString();
> -        String replStr = replacement.toString();
>          int j = indexOf(tgtStr);
>          if (j < 0) {
>              return this;
>          }
> +        String replStr = replacement.toString();
>          int tgtLen = tgtStr.length();
>          int tgtLen1 = Math.max(tgtLen, 1);
>          int thisLen = length();
>
> --
Sent from my phone