Re: Method.equals() question

[Brian Jones]

These have not been applied yet, we should do so before Friday.  Does
this look okay to everyone?

Brian

Archie Cobbs <address@hidden> writes:

> Archie Cobbs wrote:
> > The practical upshot of this is that the equals() methods that currently
> > exist in Classpath for Field and Constructor need to be fixed. But the
> > fixes could use "obj1.getDeclaringClass() == obj2.getDeclaringClass()"
> > as part of the test.
> > 
> > Method also needs to be fixed, but for a different reason (it doesn't
> > compare the return types as it should (referring to Classpath 0.05)).
> 
> Below are my patches (against 0.05) for fixing these equals() methods.
> 
> Cheers,
> -Archie
> 
> __________________________________________________________________________
> Archie Cobbs     *     Precision I/O      *     http://www.precisionio.com
> 
> diff -ur 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Constructor.java
>  ./Constructor.java
> --- 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Constructor.java
>      Fri Oct 25 16:02:26 2002
> +++ ./Constructor.java        Mon Mar  3 12:51:12 2003
> @@ -38,6 +38,8 @@
>  
>  package java.lang.reflect;
>  
> +import java.util.Arrays;
> +
>  /**
>   * The Constructor class represents a constructor of a class. It also allows
>   * dynamic creation of an object, via reflection. Invocation on Constructor
> @@ -160,7 +141,14 @@
>     */
>    public boolean equals(Object o)
>    {
> -    return this == o;
> +    if (!(o instanceof Constructor))
> +      return false;
> +    Constructor that = (Constructor)o; 
> +    if (this.getDeclaringClass() != that.getDeclaringClass())
> +      return false;
> +    if (!Arrays.equals(this.getParameterTypes(), that.getParameterTypes()))
> +      return false;
> +    return true;
>    }
>  
>    /**
> diff -ur 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Field.java
>  ./Field.java
> --- 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Field.java
>    Fri Oct 25 16:02:26 2002
> +++ ./Field.java      Mon Mar  3 12:47:44 2003
> @@ -135,7 +125,16 @@
>     */
>    public boolean equals(Object o)
>    {
> -    return this == o;
> +    if (!(o instanceof Field))
> +      return false;
> +    Field that = (Field)o; 
> +    if (this.getDeclaringClass() != that.getDeclaringClass())
> +      return false;
> +    if (!this.getName().equals(that.getName()))
> +      return false;
> +    if (this.getType() != that.getType())
> +      return false;
> +    return true;
>    }
>  
>    /**
> diff -ur 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Method.java
>  ./Method.java
> --- 
> /home/archie/classpath/classpath-0.05/vm/reference/java/lang/reflect/Method.java
>   Wed Dec 18 02:28:29 2002
> +++ ./Method.java     Mon Mar  3 12:52:48 2003
> @@ -38,6 +38,8 @@
>  
>  package java.lang.reflect;
>  
> +import java.util.Arrays;
> +
>  /**
>   * The Method class represents a member method of a class. It also allows
>   * dynamic invocation, via reflection. This works for both static and
> @@ -144,57 +136,25 @@
>    /**
>     * Compare two objects to see if they are semantically equivalent.
>     * Two Methods are semantically equivalent if they have the same declaring
> -   * class, name, and parameter list.  This ignores different exception
> -   * clauses or return types.
> +   * class, name, parameter list, and return type.
>     *
>     * @param o the object to compare to
>     * @return <code>true</code> if they are equal; <code>false</code> if not
>     */
>    public boolean equals(Object o)
>    {
> -      // Implementation note:
> -      // The following is a correct but possibly slow implementation.
> -      //
> -      // This class has a private field 'slot' that could be used by
> -      // the VM implementation to "link" a particular method to a Class.
> -      // In that case equals could be simply implemented as:
> -      //
> -      // if (o instanceof Method)
> -      // {
> -      //    Method m = (Method)o;
> -      //    return m.declaringClass == this.declaringClass
> -      //           && m.slot == this.slot;
> -      // }
> -      // return false;
> -      //
> -      // If a VM uses the Method class as their native/internal 
> representation
> -      // then just using the following would be optimal:
> -      //
> -      // return this == o;
> -      //
> -      if (o == null)
> -        return false;
> -
> -      if (!(o instanceof Method))
> -        return false;
> -
> -      Method m = (Method)o;
> -      if(!name.equals(m.name))
> -       return false;
> -
> -      if(declaringClass != m.declaringClass)
> -       return false;
> -
> -      Class[] params1 = getParameterTypes();
> -      Class[] params2 = m.getParameterTypes();
> -      if(params1.length != params2.length)
> -       return false;
> -
> -      for(int i = 0; i < params1.length; i++)
> -       if(params1[i] != params2[i])
> -           return false;
> -
> -      return true;
> +    if (!(o instanceof Method))
> +      return false;
> +    Method that = (Method)o; 
> +    if (this.getDeclaringClass() != that.getDeclaringClass())
> +      return false;
> +    if (!this.getName().equals(that.getName()))
> +      return false;
> +    if (this.getReturnType() != that.getReturnType())
> +      return false;
> +    if (!Arrays.equals(this.getParameterTypes(), that.getParameterTypes()))
> +      return false;
> +    return true;
>    }
>  
>    /**
> 
> 
> _______________________________________________
> Classpath mailing list
> address@hidden
> http://mail.gnu.org/mailman/listinfo/classpath
> 

-- 
Brian Jones <address@hidden>