Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to compare Comparable property values using compareTo() instead of equals() #5

Open
qualidafial opened this issue Sep 18, 2014 · 1 comment

Comments

@qualidafial
Copy link

While testing some database persistence, I've found that what I write out to the datastore and what gets read back from it is not always in the same format.

Example: write out price as a BigDecimal("0"), but when read back in I get BigDecimal("0.00"). Or writing out a java.util.Date, and the persistence layer reads it in as a java.sql.Timestamp.

Though the values they represent are effectively equivalent, BigDecimal.equals() returns false if the values have different scales. Timestamp.equals(Date) will always return false, but Date.equals(Timestamp) will return true if the represented time is the same.

Can we get a feature to control how property values are compared for equality?

Proposal: introduce a new property to @Property annotation:

@interface Property {
...
PropertyEquivalence equivalenceMethod default PropertyEquivalence.EQUALS;
}

enum PropertyEquivalence { EQUALS, COMPARE_TO; }

When equivalence method is COMPARE_TO, then the Pojomator should sanity check that 1) the property policy does not include hash coding, and 2) the property type is Comparable,

In the meantime, I will keep calling BigDecimal.setScale, and converting Dates to Timestamps in setters. :(

@irobertson
Copy link
Owner

irobertson commented Nov 17, 2018

Notably, this is a special case of a more general feature, which is the ability to specify arbitrary ways of computing property value equality. One might, for example, wish to compare to Strings using equalsIgnoreCase. Issue 9 presents another use case.

Notably, in either case, it's not enough to override how property equality is determined; one must also override hashCode computation. While we could in theory only allow such a feature for properties not contributing to the hashCode computation, that approach feels likely to cause confusion.

I believe that some time ago, someone proposed an idea of an interface that could check equality and generate hash codes for a type; for good measure, it could allow a toString override as well. We could then let people specify the class to use; it would not be checked at compile time that the class had a no-argument constructor, or that its generic parameter matched the annotated property. Of course, the compiler would also be unable to verify that the class followed the contract that equality implies hashCode equality.

Notably, if tasked to write a class that provides some sort of equivalence check for a type, as well as a compatible hashCode computation, it seems that the most likely approach would be to first "canonicalize" the property value, and then use normal hashCode and equals methods. For BigDecimal, one might call stripTrailingZeros(); for Dates, one could call geTime, and for Strings, toLowerCase().

With that in mind, there is an option that exists today. One cane provide a "fake" property method, which can be private, which exposes a canonical version of the property, suitable for the desired equals and hashCode behaviors. The main downsides I see to this are:

  1. It forces one or two additional annotations on classes that otherwise would just use @AutoProperty - the original property needs to be annotated to use PojomaticPolicy.NONE, and of field detection is being used, the converter method would also require annotation.
  2. In some cases, this could make the resulting equals implementation notably less efficient - it's cheaper to compare to strings with equalsIgnoreCase than it is to call toLowerCase on both instances, especially if they are unequal within the first few characters. Depending on the use case, that penalty might or might not be significant.

If the performance penalty is significant, there is yet another option: tell pojomatic to skip the relevant properties, but then add custom checks for them in the classes actual equals method, which could delegate to Pojomatic for the "normal" properties:

@AutoProperty
public class Foo {
  ....
  @Property(policy=PojomaticPolicy.NONE)
  private BigDecimal size;

  @Override
  public boolean equals(Object other) {
    return Pojomatic.equals(this, other) 
      && (this.size == null ? other.size == null : this.size.compareTo(other.size) == 0)
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants