12 comments
Comment actions Permalink

You should file this on the issue tracker at jetbrains.net/jira.

How about adding a "Adding class to Set/Map" that does not implement
equals() and hashCode() ?




0
Comment actions Permalink


Way too many false positives. Inheriting equals() and hashCode() from Object is the standard way of saying "Objects of this class are entities, and are distinguished by identity". There's no problem putting such objects into Sets or Maps, the semantics are well defined, clear, and useful.

In short, I just don't see the use of this inspection.

--Dave Griffith

0
Comment actions Permalink

If you add objects in to a set/map, that dont implement equals() and hashCode(), you get some real odd results.

If you want an example, I can email one.

0
Comment actions Permalink


If you add objects to a set/map that implement precisely one of equals() and hashCode(), you get odd results. If you add objects to a set/map that implement neither, you get good solid identity semantics. There's already a built-in inspection for implementing either both of equals() and hashCode() or neither. That would seem to be good enough.

--Dave Griffith

0
Comment actions Permalink

If you add objects in to a set/map, that dont implement equals() and hashCode(), you get some real odd results.


Strange, this 80% of our objects stored in collections and maps do not
overwrite equals() and hashCode(). We never got odd results.

Tom

0
Comment actions Permalink

As Dave commented, the odd cases are when you override exactly one of them.

0
Comment actions Permalink

Thomas Singer (MoTJ) schrieb:
>> If you add objects in to a set/map, that dont implement equals() and
>> hashCode(), you get some real odd results.


Strange, this 80% of our objects stored in collections and maps do not
overwrite equals() and hashCode(). We never got odd results.

Tom


If you favor trees over hashes, you should only need equals(). But it
makes sense to always implement hashCode() as well, just in case ..

Mike

0
Comment actions Permalink

And this existing inspection warnings me when I put those objects in a map/set?

If so, where is it located (Ill go turn it on).
I dont want a warning for every single object that doesnt implement equals+hashCode, only those that I try to stick in a map/set

Nick

0
Comment actions Permalink

NP> And this existing inspection warnings me when I put those objects in
NP> a map/set?

Not specifically no.

NP> I dont want a warning for every single object that doesnt implement
NP> equals+hashCode, only those that I try to stick in a map/set

It warns you if you implemented equals without hashcode or vice-versa. That
shouldn't happen and should catch the real mistakes.

Inserting a class not overriding equals+hashcode into a set/map may be not
very useful. Inserting a class implementing only one of them is a source
of hard to find bugs.

NP> If so, where is it located (Ill go turn it on).

In Settings -> Errors filter by equals. This inspection is under Probable
Bugs.



0
Comment actions Permalink

>And this existing inspection warnings me when I put those objects in a map/set?

No, the existing inspection flags classes that implement precisely one of equals() and hashCode(). If you implement both, or neither, the objects are perfectly safe for inclusion in a map or set, and the inspection won't flag them.

>I dont want a warning for every single object that doesnt implement equals+hashCode, only those that I try to stick in a map/set

Extremely dangerous coding style, I would suggest. At the point of inclusion to the set or map, all that can be really told about the object to be added is the type of the argument, which may be a supertype of the object's type. If I'm practicing component-oriented programming, for instance, all I would ever know about the object added would be an interface, not it's implementation class. In circumstances like that, your suggested inspection would never trigger, even if the underlying implementation classes had overridden equals() but not hashCode().

That said, such an inspection could find some bugs in cases where you're adding busted library objects to a collection. The existing inspection wouldn't find those, since inspections don't get run on library classes. That's a pretty small bug class, and one I've never seen in the wild, but there are some busted libraries out there. If anyone can tell me that they've spent time fighting such a bug, I'll add this to the list for inclusion. Alternatively, if some kind soul codes up the inspection and mails it to me, I'll code read, test, and include it.

Summing up, it is my understanding that the proposed new inspection would detect the addition of objects to a java.util.Set or use as keys to java.util.Map, where that object has a concrete class type (i.e. isn't an array, primitive, interface, or abstract class) and directly implements precisely one of equals() or hashCode(). Is that a fair description?

--Dave Griffith

0
Comment actions Permalink

I requested this inspection after having brought down our production schedule for this very reason. {:-)

Having the existing warning enabled (one or the other, but not both or neither) would light my editor up like a Christmas tree.

Being warned that I tried to put one of our busted framework/model objects into a Set/Map would have saved some pain. {:-)

0
Comment actions Permalink

And in answer to your final question, yes, you hit the nail on the head.

0

Please sign in to leave a comment.