kamikat / moshi-jsonapi

JSON API v1.0 Specification in Moshi.
MIT License
156 stars 34 forks source link

Equals contract for data classes #58

Closed p-fischer closed 7 years ago

p-fischer commented 7 years ago

I want to write a valid equals method for my class extending from Resource taking into account its attributes. The issue is that its superclass ResourceIdentifier already overwrites equals().

One of the requirements for equals is symmetry. This is where I fail. Let's say my concrete instance is c an instance of ResourceIdentifier is s. If the both have equal type and id then s.equals(c) == true, however c.equals(s) == false.

There is a good read on that issue: http://www.artima.com/lejava/articles/equality.html

I see three options:

  1. I realize I do not need an extra equals method. (That would imply I do not consider the attributes of my concrete class to contribute to its state.)
  2. ResourceIdentifier does not override equals at all (I have no idea if it is required.)
  3. Using the canEquals approach as mentioned in the article to solve the problem.
kamikat commented 7 years ago

This is an ugly hack for indexing included resources (#48). I've edit the class and you can try snapshot version to see if the patch solves your problem.

kamikat commented 7 years ago

I've just released the patch to 3.2.2 🍻

p-fischer commented 7 years ago

There might be a misunderstanding. I want to have my own data class extending from Resource. In that data class I want to implement equals and hashcode in a way the equals contract is satisfied. That is impossible right now since ResourceIdentifier, which is a transitive superclass, already implements equals / hashcode. Since my data class introduces new attributes, thus extends the classes state, I cannot implement equals in a way the symmetry requirement can be satisfied.

I know that's quite subtle, but this testing library pointed me towards it.

kamikat commented 7 years ago

There you can override equals and hashCode function and use your own implementation.

p-fischer commented 7 years ago

That's what I did. Still the symmetry of the equals contract is not satisfied when I compare an instance of my own data class and an instance of ResourceIdentifier. See my first comment.

I just wanted to point that out. But you can consider it low prio ;)

kamikat commented 7 years ago

Umm... do you mean to make a.equals(b) == true as long as a and b are both instance of ResourceIdentifier with the same id and type?

p-fischer commented 7 years ago

I'd suggest the canEqual approach from my link above. That would cause a.equals(b) == false and b.equals(a) == false if a is a superclass of b.

kamikat commented 7 years ago

I read the article again. Generally, the newly implemented ResourceIdentifier conforms to equality contract described there.