8058779: Faster implementation of String.replace(CharSequence, CharSequence)

[Remi Forax]

I agree with the others the code is more readable.

There is a slightly difference between the current behavior and the 
behavior of the proposed code,
   "foo".replace("bar", null)
should throw a NPE because replacement is null but the proposed code 
will return "foo"
because replacement.toString() is called after the short-circuit test (j 
< 0).

so the first part of the code should be:
   String starget = target.toString();  // implict nullcheck
   String srepl = replacement.toString();  // implicit nullcheck
   int j = indexOf(starget);
   if (j < 0) {
     return this;
   }
   ...

also 'i' can be initialized just before the 'do', there is no point to 
initialize it before.

To finish, the two 'final' are useless here but i suppose it's a matter 
of style :)

cheers,
Rémi


On 05/31/2015 04:19 AM, Ivan Gerasimov wrote:
> Hi everyone!
>
> Here's another webrev, in which replace() is implemented with 
> StringBuilder.
> On my benchmark it is almost as fast as the version backed with 
> arrays, but this variant is much shorter.
>
> Credits to Sherman for combining the general algorithm with the case 
> of empty target.
>
> Comments, further suggestions are welcome!
>
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8058779
> WEBREV: http://cr.openjdk.java.net/~igerasim/8058779/04/webrev/
>
> Sincerely yours,
> Ivan
>