pojomatic-users

[Pojomatic-users] equals-hashCode contract

[Ian R. <ian...@gm...>]

I'm looking into addressing bug
2845939<https://sourceforge.net/tracker/?func=detail&aid=2845939&group_id=239113&atid=1108645>(equals
doesn't play well with inheritance).  The basic strategy for
implementing doEquals(instance, other) in the Pojomator for class Pojoshould be:


   - if other.getClass() is a proper subclass of Pojo, then invoke
   other.equals(instance).
   - Otherwise, verify that other.getClass()is a subclass of the highest
   class in Pojo's heirarchy which contains all properties which are
   included in the EQUALS role for Pojo.  If so, then do the standard
   property-by-property equals check.
   - Otherwise, return false.

Taken together, these mean that two instances are equals() to each other iff
they share the same set of properties with the EQUALS role, and those
proeprties are equal to each other.  Note that if a parent class has a
method getFoo() which is not in the EQUALS role, and two distinct subclasses
add getFoo() to the EQUALS role, these will be seen as distinct properties.

Unfortunately, I'm realizing that another sticky issue is the relationship
between equals and hashCode.  Currently, we have the rule that if an
override of a method chosen as a property for a given role (equals, toString
or hashCode) is overridden, and the override is also chosen as a property
for the same role, then only the parent method will be used.  Unfortunately,
we do this on a role-by role basis, without paying attention to
interactions.  This can cause a problem when a method is included in the
equals computation in a parent class, and then a child class adds hashCode
to it's roles.  The result is that an instanceof the parent class might be
equals() to an instance of the child class, but the two instances will have
different hashCode values.

I can see a few different ways of handling this.  The easiest is to simply
declare that if a method property has role EQUALS and not HASH_CODE, then
subclasses can not add the HASH_CODE role to the method.  While correct, it
is somewhat overrestrictive.  If the subclass has also added additional
properties, then there is no risk of instances of the parent class being
equals()to instances of the child class, so allowing parent properties to
take on the additional HASH_CODE role would be fine.  Implementing this is a
bit more complicated, of course.  I might start by doing the easy thing;
becoming more permisive in the future allowed.

In either case, a question arises of what should happen if someone requests
adding a property to a HASH_CODE role where it previously had only the
EQUALS role (ignoring the TO_STRING role here).  One option is to silently
ignore.  A second is to log a complaint.  A third is to fail fast with a
RuntimeException.  I lean towards the third - it's almost certainly a
programmer error.  The only spot of roughness I can see here would be if a
project depended on two libraries, with one of the libraries extending a
pojomated class of another library.  Updates to the library containing the
parent class could suddenly cause the child class to be in error, but the
error might be tolerable.  A RuntimeException approach would make life
unpleasant for the project.  On the other hand, I suspect this would be an
exceedingly rare case.  In any event, we can throw an exception today and
then become more lenient in the future; going the other direction would not
be backwards compatible.

  - Ian

Re: [Pojomatic-users] equals-hashCode contract

[Ian R. <ian...@gm...>]

On Sat, Aug 29, 2009 at 12:07 PM, Ian Robertson
<ian...@gm...>wrote:

> In either case, a question arises of what should happen if someone requests
> adding a property to a HASH_CODE role where it previously had only the
> EQUALS role (ignoring the TO_STRING role here).  One option is to silently
> ignore.  A second is to log a complaint.  A third is to fail fast with a
> RuntimeException.  I lean towards the third - it's almost certainly a
> programmer error.  The only spot of roughness I can see here would be if a
> project depended on two libraries, with one of the libraries extending a
> pojomated class of another library.  Updates to the library containing the
> parent class could suddenly cause the child class to be in error, but the
> error might be tolerable.  A RuntimeException approach would make life
> unpleasant for the project.  On the other hand, I suspect this would be an
> exceedingly rare case.  In any event, we can throw an exception today and
> then become more lenient in the future; going the other direction would not
> be backwards compatible.


Another wrinkle with the RuntimeException approach is the case where a child
class annotated with @AutoProperty(autoDetect=AutoDetect.METHOD) overrides a
method from a parent class which was given a role of EQUALS but not
HASH_CODE.  Should this be an error, logged, or silently ignored?

  - Ian

Re: [Pojomatic-users] equals-hashCode contract

[Chris H. <han...@gm...>]

On Sat, Aug 29, 2009 at 12:07 PM, Ian
Robertson<ian...@gm...> wrote:
> Taken together, these mean that two instances are equals() to each other iff
> they share the same set of properties with the EQUALS role, and those
> proeprties are equal to each other.

This behavior sounds correct to me, but I'm wondering if this would be
more maintainable and/or easier to implement exactly as stated above
instead of doing non-intuitive things like calling
other.equals(instance). I will attempt to explain what I mean. We
could say that two properties are equal with respect to EQUALS if they
"originate" from the same class in the inheritance hierarchy and are
both included in the EQUALS role of the class in question. Then, two
instances are equal if their properties are equal with respect to
EQUALS for their Object.getClass(). We just need to come up with what
"originate" means precisely, in the sentence above. Let me know if I'm
not making myself clear and we can discuss this in person, at a
whiteboard.

> Currently, we have the rule that if an
> override of a method chosen as a property for a given role (equals, toString
> or hashCode) is overridden, and the override is also chosen as a property
> for the same role, then only the parent method will be used.

It seems to me like the overriding implementation (i.e. the child
class) should always be called instead of the parent method. Am I
missing something here?

> I can see a few different ways of handling this.  The easiest is to simply
> declare that if a method property has role EQUALS and not HASH_CODE, then
> subclasses can not add the HASH_CODE role to the method.  While correct, it
> is somewhat overrestrictive...  I might start by doing the easy thing;
> becoming more permisive in the future allowed.

I think that makes sense.

> In either case, a question arises of what should happen if someone requests
> adding a property to a HASH_CODE role where it previously had only the
> EQUALS role (ignoring the TO_STRING role here).  One option is to silently
> ignore.  A second is to log a complaint.  A third is to fail fast with a
> RuntimeException.  I lean towards the third - it's almost certainly a
> programmer error.  The only spot of roughness I can see here would be if a
> project depended on two libraries, with one of the libraries extending a
> pojomated class of another library.  Updates to the library containing the
> parent class could suddenly cause the child class to be in error, but the
> error might be tolerable.  A RuntimeException approach would make life
> unpleasant for the project.  On the other hand, I suspect this would be an
> exceedingly rare case.  In any event, we can throw an exception today and
> then become more lenient in the future; going the other direction would not
> be backwards compatible.

I agree and I also like the RuntimeException approach. The message
must be exceedingly clear, however.

> Another wrinkle with the RuntimeException approach is the case where a child
> class annotated with @AutoProperty(autoDetect=AutoDetect.METHOD) overrides a
> method from a parent class which was given a role of EQUALS but not
> HASH_CODE.  Should this be an error, logged, or silently ignored?

I think it should be a RuntimeException for the same reasons mentioned above.

-Chris