[jira] Commented: (AVRO-182) hashCode and equals are not consistent with compareTo

["Philip Zeyliger (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/AVRO-182?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774529#action_12774529 ] 

Philip Zeyliger commented on AVRO-182:
--------------------------------------

A few comments.  I think the equals() thing is a bug.  Everything else is commentary.

{noformat}
    public boolean equals(Object that) {
      return this.compareTo((Record)that) == 0;
    }
// also: return this.compareTo((Array)that) == 0;
{noformat}
Doesn't this throw a runtime cast exception?  equals() should check type-compatibility first.  You should add a quick test ("assertFalse(o1.equals(new Object()))").

bq. public int hashCode(Object o, Schema s) 

Do you feel it beneficial to include the hashcode of the schema in the hashcode as well?

bq. hashCode = 31*hashCode + hashCode(e, elementType);

The google collections library does something clever (I think) by delegating to java.util.Arrays.hashCode().  That hides the "31*foo + bar" logic.  (Arrays.hashCode() in 1.5.0 is "31*foo + (bar ^ (bar >>> 32))" if I'm reading it right.  Either way, it seems like extracting the logic for combining two hash codes into a function might be nice.

{noformat}
  /**
   * Generates a hash code for multiple values. The hash code is generated by
   * calling {@link Arrays#hashCode(Object[])}.
   *
   * <p>This is useful for implementing {@link Object#hashCode()}. For example,
   * in an object that has three properties, {@code x}, {@code y}, and
   * {@code z}, one could write:
   * <pre>
   * public int hashCode() {
   *   return Objects.hashCode(getX(), getY(), getZ());
   * }</pre>
   *
   * <b>Warning</b>: When a single object is supplied, the returned hash code
   * does not equal the hash code of that object.
   */
  public static int hashCode(Object... objects) {
    return Arrays.hashCode(objects);
  }
{noformat}

bq. GenericArray a = (GenericArray)o;

You chose GenericArray to delegate to GenericData, versus the other way around.  For my own edification (mostly to further my understanding of how the Generic* stuff works), why choose that direction and not the other?

> hashCode and equals are not consistent with compareTo
> -----------------------------------------------------
>
>                 Key: AVRO-182
>                 URL: https://issues.apache.org/jira/browse/AVRO-182
>             Project: Avro
>          Issue Type: Bug
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-182.patch
>
>
> Java's specific and generic APIs implement compareTo according to the schema, where some fields might be ignored.  To be consistent, fields that are ignored when comparing for ordering should also be ignored when comparing for equality and for computing hashCodes.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.