phax / jcodemodel

A heavily extended fork of the com.sun.codemodel (from 2013/09)
Other
92 stars 34 forks source link

equals/hashCode for IJExpressions #20

Closed leventov closed 9 years ago

leventov commented 10 years ago

Seems that implementing equals() and hashCode() methods for IJExpression subclasses is an absolute requirement for starting any analysis (#18). What way of implementing this do you prefer for the lib? Some particular auto generation utility, such as from Apache Commons, or Lombok, or Google's auto-value? Generating from Eclipse/Intellij? Or writing by hand?

(Of cause I would be very grateful if you implement this, but if don't want to, I will be forced to do it myself, and will try to accout your advice.)

phax commented 10 years ago

This is a very good question - I would not like to add additional libraries for this, so we will stick with "write by hand" :) But I do have a simple "HashCodeGenerator" in one of my other projects, which I would like to reuse. It is checked into the "util" package.

Example for a class with base class Object:

@Override
 public int hashCode ()
 {
   return new HashCodeGenerator (this).append (member1).append (member2).getHashCode ();
 }

Example for a class which is derived from a class different than Object:

@Override
 public int hashCode ()
 {
   return HashCodeGenerator.getDerived (super.hashCode ()).append (member3).append (member4).getHashCode ();
 }

Concerning equals, I don't have a real helper because there are four different versions of equals required, depending on the scope of the class:

  1. class is not final and it's base class is Object
  2. class is final and it's base class is Object
  3. class who's base class is NOT Object

Here are the examples for all cases:

public class Case1 {
  private String s;
  private int i;
  public boolean equals (Object o) {
    // always the first clause
    if (o == this)
     return true;
    // Parameter is null or from a different implementation class
    if (o == null || !getClass ().equals (o.getClass ()))
      return false;
    // Same class and parameter not null - start comparing
    Case1 rhs = (Case1) o;
    return EqualsUtils.equals (s, rhs.s) && EqualsUtils.equals(i, rhs.i);
  }
}
public final class Case2 {
  String s;
  int i;
  public boolean equals (Object o) {
    // always the first clause
    if (o == this)
     return true;
    // Parameter is null or from a different implementation class
    // Theoretically the line from Case1 could also be used here, but I think this is more readable
    if (!(o instanceof Case2))
      return false;
    // Same class and parameter not null - start comparing
    Case2 rhs = (Case2) o;
    return EqualsUtils.equals (s, rhs.s) && EqualsUtils.equals(i, rhs.i);
  }
}
public class Case3 extends Case1 {
  private int x;
  public boolean equals (Object o) {
    // always the first clause
    if (o == this)
     return true;
    // Parameter is null or from a different implementation class
    if (!super.equals (o))
      return false;
    // Same class and parameter not null - start comparing
    Case3 rhs = (Case3) o;
    return EqualsUtils.equals (s, rhs.s) && EqualsUtils.equals(i, rhs.i);
  }
}

So it can be drilled down to Case1 and Case3 if you want. Btw. EqualsUtils is a small utility class in the util package, that performs null-safe-equals.

Do you think this is reasonable?

leventov commented 10 years ago

equals static methods would better have a different name, not colliding with Object.equals() to allow static import.

Could you also provide a method accepting varargs to compute hashCode? For example HashCodeGenerator.hashCode(Object srcObject, Object... members) and HashCodeGenerator.hashCode(int superHashCode, Object... members)

phax commented 10 years ago

I would go for isEqual - does that sound reasonable?

Concerning hashCode: I personally totally dislike the boxing concept, but I see the added value on this. So I will do it on Monday as you suggested.

On 20. September 2014 08:06:15 MESZ, Roman Leventov notifications@github.com wrote:

equals static methods would better have a different name, not colliding with Object.equals() to allow static import.

Could you also provide a method accepting varargs to compute hashCode? For example HashCodeGenerator.hashCode(Object srcObject, Object... members) and HashCodeGenerator.hashCode(int superHashCode, Object... members)


Reply to this email directly or view it on GitHub: https://github.com/phax/jcodemodel/issues/20#issuecomment-56258119

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

leventov commented 10 years ago

Yes, that's ok.

Thank you, I won't start implementing anything until monday, too.

phax commented 10 years ago

All done. Additionally the isEqual(Object,Object) method was extended for arrays :)