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
