Mark Wielaard wrote:
Hi
On Sat, 2003-11-22 at 13:06, Jeroen Frijters wrote:
The fix is actually incorrect. containsValue() should call contains() because containsValue() is a new method (since 1.2) and contains() exists since 1.0. Older code may have overridden contains() and this should work with newer code that calls containsValue().
Urgh. Yes, you are right. Wrote an additional test case for this: gnu.testlet.java.util.Hashtable.ContainsHash (it currently fails).
Thanks for pointing that mistake out. I've attached an updated version of the patch.
2003-11-28 Dalibor Topic <[EMAIL PROTECTED]>
Reported by: Jim Pick <[EMAIL PROTECTED]>
* libraries/javalib/java/util/Hashtable.java
(internalcontainsValue): New method.
(contains) Delegate to internalContainsValue.Reported by: Mark Wielaard <[EMAIL PROTECTED]>
* libraries/javalib/java/util/Hashtable.java
(contains): Improved comment.Reported by: Jeroen Frijters <[EMAIL PROTECTED]>
* libraries/javalib/java/util/Hashtable.java
(containsValue): Delegate to contains(Object) to make sure older
code overwriting it continues to work.I'm not sure how to deal with mutliple people's bug reports being fixed in this patch in the ChangeLog entry, so I'd appreciate a hint from the ChangeLog police.
As I've argued before, in cases such as these (multiple virtual methods that do the same thing), we must use the same delegation as the Sun implementation to be compatible.
Problem is that this is not always easy to know. So we depend on bug reports and then have to write a Mauve test if we think that following such behavior is beneficial. But the "newer calls older" is a guideline that will mostly work (I hope).
Yes, that makes the most sense to me, too. Stuart Ballard started working on a 'override compatibility' test suite for collection classes. See [1] for details.
cheers, dalibor topic
[1] http://www.kaffe.org/pipermail/kaffe/2003-May/042181.html
--- /var/tmp/PROJECTS/classpath/java/util/Hashtable.java Wed Nov 12 21:56:20
2003
+++ kaffe/libraries/javalib/java/util/Hashtable.java Fri Nov 28 17:11:51 2003
@@ -333,7 +333,11 @@
*/
public synchronized boolean contains(Object value)
{
- return containsValue(value);
+ /* delegate to non-overridable worker method
+ * to avoid blowing up the stack, when called
+ * from overridden contains[Value]() method.
+ */
+ return internalContainsValue(value);
}
/**
@@ -349,6 +353,25 @@
* @since 1.2
*/
public boolean containsValue(Object value)
+ {
+ /* delegate to older method to make sure code overwriting it
+ * continues to work.
+ */
+ return contains(value);
+ }
+
+ /**
+ * Returns true if this Hashtable contains a value <code>o</code>, such that
+ * <code>o.equals(value)</code>. This is an internal worker method
+ * called by <code>contains()</code> and <code>containsValue()</code>.
+ *
+ * @param value the value to search for in this Hashtable
+ * @return true if at least one key maps to the value
+ * @see #contains(Object)
+ * @see #containsKey(Object)
+ * @throws NullPointerException if <code>value</code> is null
+ */
+ private boolean internalContainsValue(Object value)
{
for (int i = buckets.length - 1; i >= 0; i--)
{
_______________________________________________ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
