On Oct 15, 2014, at 9:54 AM, Aleksey Shipilev <aleksey.shipi...@oracle.com> wrote:
> 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! > I can do it later today. Paul.
