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.
On Wed, Mar 1, 2017 at 4:37 PM Christoph Dreis <christoph.dr...@freenet.de>
wrote:
> Hey,
>
> I just noticed a small improvement for String.replace(CharSequence,
> CharSequence) in case the target sequence is not found.
>
> There is the possibility of an early-out in case there is nothing to
> replace. Yet there is a call to replacement.toString();
> While this is usually not a problem in case of String parameters, a
> StringBuilder - or other more "dynamic" CharSequence.toString()
> implementations - will likely produce unnecessary allocations here.
>
> I think the JDK could prevent that with the given patch below.
>
> I would be very happy if this is sponsored for JDK 10.
>
> Cheers,
> Christoph
>
> ===== 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,11 @@
> */
> public String replace(CharSequence target, CharSequence 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
