And by "deep equals" I mean equivalent behavior to https://docs.oracle.com/javase/8/docs/api/java/util/Arrays.html#deepEquals-java.lang.Object:A-java.lang.Object:A-
at least that is what AutoValue has been doing. On Mon, May 16, 2016 at 8:05 PM, Kevin Bourrillion <[email protected]> wrote: > On Mon, May 16, 2016 at 11:20 AM, Brian Goetz <[email protected]> > wrote: > > Kevin say: >> >> However, it would be very sad if it does not solve a second problem at >>> the same time, because it can. Even Java developers who are content with >>> the performance foibles of their existing "value-based classes" are >>> constantly irritated by the burden and bug-prone nature of >>> writing/maintaining such classes. >>> >> >> We have two choices in front of us regarding equality on value types: >> >> 1. What does `==` on values do? Choices include a bitwise comparison, a >> componentwise comparision (identical to bitwise unless there are float >> members), or invoking the `equals()` method. >> > > I'd recommend that this fail to compile. > > When I come across this code: > > a == b > > I want to know what that code is doing, and I want to know that it's > correct. So I jump to the definitions of a and b. If I see that they're > ints, I'm done. > > If, however, they are SomeType, today I know that I am looking at > reference equality. I'd like that to continue to be the case. People like > to know what they're looking at. I wouldn't like to have to dig into the > definition of *SomeType* (which may be "far away") just to know what sort > of equality I'm looking at. > > (Note that even in the case of enums, where reference equality and value > equality are identical, our internal coding guidelines still recommend > always using .equals() for this reason. Then you have code that is not only > correct, but is *obviously* correct. Not code that you have to look up a > foreign definition to know whether it's correct or not. That's useful, and > what's the cost? Just a bit of extra text on the screen. This is Java.) > > If we insist that == do something, then I'd recommend it be identical to > equals(). We should assume that the person who wrote equals() made their > choices for a reason. It's impossible to know whether exposing > componentwise equality would make sense for a type or not. Not that I love > this option; it makes me a little squeamish for an operator to invoke user > code. My preference is to not compile. > > > 2. What is the default for the `equals()` method? The obvious choice is >> a componentwise comparision, but Kevin has (strongly) suggested we consider >> a deep comparison, calling .equals() on any reference members. (Note that >> the two choices collapse to the same thing when there are no reference >> members.) >> > > Not just equals(), but deepEquals(), as proposed by Bill Pugh. > > > >> For values whose members are value-ish objects themselves (e.g., String, >> Optional, LocalDateTime), the deep-equals is obviously appealing -- a tuple >> of (int, date) makes total sense to compare using .equals() on the date. >> On the other hand, for values whose members are references to mutable >> things (like lists), it's not as obvious what the right default is. >> > > When you put it this way it seems even more clear that the "value-ish" > case is the one to design toward; the one that should not require the user > to add custom code. > > Of course we know that many different people implement equals() according > to many different goals. But I've become convinced there is only *one* > way to look at equals() that *actually makes sense*, and around which > APIs can and have been and should be designed. And that is to say that *equals > means interchangeable*. 99% of the time, you should never care which of > two equal instances you have. If you do, either you are doing some > low-level nuts-and-bolts far away from business logic (and this should > apply to extremely few people), or you implemented equals() wrong. > > Lots of equals() methods *are* implemented wrong and many of those can > never be fixed. However, I think it would be very questionable to design > toward such cases. *Those* should be the cases where you have to write > custom code. The default should be "equals means interchangeable". > > Mutable lists are a fuzzy case, but they tend to get away with their > "wrong" behavior because of how often we *treat* them as immutable (er, > meaning, we *don't mutate them*). Still, here we strongly encourage using > ImmutableList as much as possible for reasons like this and all the > well-known others. > > > So, Kevin -- please make your case! Please share your experience with >> tools like AutoValue, and if you can, data on how frequently (and why) >> equals() is underridden on auto values in the Google codebase? >> > > Okay. To refresh, we have a code generator for "value-based classes" > because they're that much of a pain. As of today, it's used 17,400 times in > our codebase. Its equals() behavior is not configurable; it always uses > deep value equality unless the user replaces it with their own custom > equals() implementation. > > I've researched how often Google users are doing this and why. I decided > to manually review *all* instances of this, but at 70% of the way through > I'm getting hungry, so I'll just crudely extrapolate from what I did get to: > > - About 30 classes customize equals so as to ignore some fields. > (These are a little bit suspicious according to the "equals means > interchangeable" maxim; I'd probably suggest they redesign a bit and keep > their proper values separate from their other stuff. In some cases the > ignored fields are derived, so it's fine, although in some of *those* I > consider going out of their way to ignore the field in equals as just a > hyperoptimization.) > - About 25 classes do it so that they can substitute some alternate > determination of equivalence for at least one field. Most of these are > working around a field's what-I'd-call-broken equals() behavior. None were > doing this to switch to identity comparison. > - About 15 are doing very strange and suspicious things and *deserve > whatever they get*. > - About 10 seem to be just plain misunderstandings. > - 1 or 2 customize it so that they can also be equal to other sibling > types implementing a common interface. Okay. > > In all, fully 99.5% of usages accept the default behavior. > > So, for evidence that reference equality or even shallow value equality is > what anyone wants, out of 17,400 usages I've come up empty. (Granted, > there may be users who want it and therefore just don't use AutoValue at > all. But Googlers are very good at complaining when a tool doesn't do quite > what they want, and we're not getting that either.) > > HTH > > > -- > Kevin Bourrillion | Java Librarian | Google, Inc. | [email protected] > -- Kevin Bourrillion | Java Librarian | Google, Inc. | [email protected]
