Bug 70602 - Why is equals(..) method of IBindings not implemented?
Summary: Why is equals(..) method of IBindings not implemented?
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.1 M3   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-22 04:42 EDT by Markus Keller CLA
Modified: 2004-11-02 10:24 EST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2004-07-22 04:42:16 EDT
The Javadoc of IBindings#equals(Object obj) tells how clients should compare
IBindings. Is there a reason why the equals method is not implemented in
implementations of the interface?

JDT UI currently uses an equals(IBinding b1, IBinding b2) method in an internal
helper class to achieve the desired behavior. However, we cannot e.g. put
bindings into a HashSet, because bindings from different clusters are not
considered equal.
Comment 1 Jim des Rivieres CLA 2004-08-25 19:09:54 EDT
As spec'd, bindings are inexpensive for clients that are only dealing with one 
cluster of them. These clients do not need keys (IBinding.getKey()). Judging 
equality between clusters of bindings involves computing keys, which is 
somewhat more expensive. This is why more complex comparisons are left to 
clients rather than being built in to IBinding.equals(Object).

Is the inconvenience to clients severe enough to change how this method is 
spec'd?
Comment 2 Dirk Baeumer CLA 2004-08-26 14:07:21 EDT
Refactoring typically deals with clusters of bindings. So we wrote special 
equals methods which we use all over the place. Currently putting bindings 
into sets or hash maps isn't possible. 
Comment 3 Dirk Baeumer CLA 2004-09-16 06:09:01 EDT
We discussed this during the JDT meeting in ZRH and came to the following 
concolusion:

Binding will provide a method isEqualTo which isn't based on identity (to not
break the spec).

We already have our own implementation of a hash table which delegates equals 
and hashcode to a strategy. 

Comment 4 Jim des Rivieres CLA 2004-09-16 09:12:23 EDT
Olivier,  Assign to me and I'll take care of spec.
Comment 5 Jim des Rivieres CLA 2004-09-16 13:24:28 EDT
Added IBinding.isEqualTo(IBinding binding) and implementation.
Olivier, please add appopriate tests. Thnx

	/**
	 * Returns whether this binding has the same key as that of the given
	 * binding. Within the context of a single cluster of bindings, each
	 * binding is represented by a distinct object. However, between
	 * different clusters of bindings, the binding objects may or may
	 * not be different objects; in these cases, the binding keys
	 * are used where available.
	 * 
	 * @param binding the other binding, or <code>null</code>
	 * @return <code>true</code> if the given binding is the identical
	 * object as this binding, or if the keys of both bindings are the
	 * same string; <code>false</code> if the given binding is
	 * <code>null</code>, or if the bindings do not have the same key,
	 * or if one or both of the bindings have no key
	 * @see #getKey()
	 * @since 3.1
	 */
	public boolean isEqualTo(IBinding binding);
Comment 6 Olivier Thomann CLA 2004-09-16 14:29:21 EDT
I don't see what that method is adding. This is simply a convenient method to
compare the keys. This could clearly be done by the users and it doesn't need to
be provided as an API.
As you said, you already have code to handle comparison not based on identity.
So why adding this new method?
When such things are discussed, I'd like to join the discussion.
Comment 7 Dirk Baeumer CLA 2004-09-16 14:46:58 EDT
Philippe mentioned that generating the key isn't cheap. So he suggested that 
the method can be implemented using binding internals (instead of using the 
key). We do lots of binding comparisons outside of hash tables. 
Comment 8 Olivier Thomann CLA 2004-09-20 10:51:57 EDT
Too late for M2. Changed milestone to M3.
Comment 9 Olivier Thomann CLA 2004-09-20 11:49:21 EDT
The method has already been added for M2.
A more optimized implementation will be provided for M3.
Comment 10 Olivier Thomann CLA 2004-10-05 18:10:40 EDT
Version that is not creating the keys is released.
Fixed and released in HEAD.
Regression tests added.
Comment 11 David Audel CLA 2004-11-02 10:24:08 EST
Verified for 3.1M3 with build I20041101