On 14.10.2014 21:55, Alan Bateman wrote: > On 14/10/2014 18:38, Aleksey Shipilev wrote: >> Thanks guys! >> >> And of course, I managed to do two minor mistakes in a two-line change: >> the indentation is a bit wrong, and cast to String is redundant. Here is >> the updated webrev and the changeset (need a Sponsor!): >> http://cr.openjdk.java.net/~shade/8060485/webrev.01/ >> http://cr.openjdk.java.net/~shade/8060485/8060485.changeset >> >> -Aleksey. >> > Updated version looks okay. I wonder how common it might be to call this > method with String vs. a StringBuilder, just wondering if it should > check for a String first (although the type check should be really quick > and probably wouldn't make a difference).
Thanks Alan. One of my Twitter followers has pointed this thing to me, so I presume they got hit by this quirk while calling contentEquals with String. I concur the instanceof check is very cheap, and so it does not matter to check the String first. In either case, I think the most preferred way for checking the String vs. String equality is equals(), not contentEquals(). Added you as the reviewer: http://cr.openjdk.java.net/~shade/8060485/8060485.changeset Need a sponsor to push! -Aleksey.