The collections implementations are a template of our suggested practices for people writing generic code or generifying their existing code, so it should be as clean as possible both externally and internally. ====================================================== Date: Mon, 17 May 2004 09:38:01 -0700 From: ###@###.### Subject: Re: Double fisted casting? To: ###@###.### Cc: ###@###.### ###@###.### wrote: > >> My code wouldn't compile without the double cast. >> >> Note that the code below is YOUR code, that I just found in the >> library sources. It wouldn't compile without the double cast either. >> >> > It's Sun's code, but I believe ###@###.### parameterized it. That's why I > asked him about the double cast. OK, there are two things going on here. First, the double cast is necessary because doing it as a single cast would provoke a compile-time error. The compile-time error is correct: the compiler has detected a cast from one type (EntrySet) to another (Set<Map.Entry<K,V>>) that could not ever be correct (because EntrySet extends a different, non-parameterized version of Set). The double cast hides from the compiler the fact that we are doing a cast that is not merely unsafe, but incorrect. So why do we have an incorrect cast in the code? The answer is: it was added to work around a compiler bug that has long since been fixed. The following code in HashMap.java: public Set<Map.Entry<K,V>> entrySet() { Set<Map.Entry<K,V>> es = entrySet; return (es != null ? es : (entrySet = (Set<Map.Entry<K,V>>) (Set) new EntrySet())); } private class EntrySet extends AbstractSet/*<Map.Entry<K,V>>*/ { public Iterator/*<Map.Entry<K,V>>*/ iterator() { return newEntryIterator(); } public boolean contains(Object o) { if (!(o instanceof Map.Entry)) return false; Map.Entry<K,V> e = (Map.Entry<K,V>) o; Entry<K,V> candidate = getEntry(e.getKey()); return candidate != null && candidate.equals(e); } public boolean remove(Object o) { return removeMapping(o) != null; } public int size() { return size; } public void clear() { HashMap.this.clear(); } } should be replaced by its correct version: public Set<Map.Entry<K,V>> entrySet() { Set<Map.Entry<K,V>> es = entrySet; return (es != null ? es : (entrySet = new EntrySet())); } private class EntrySet extends AbstractSet<Map.Entry<K,V>> { public Iterator<Map.Entry<K,V>> iterator() { return newEntryIterator(); } public boolean contains(Object o) { if (!(o instanceof Map.Entry)) return false; Map.Entry<K,V> e = (Map.Entry<K,V>) o; Entry<K,V> candidate = getEntry(e.getKey()); return candidate != null && candidate.equals(e); } public boolean remove(Object o) { return removeMapping(o) != null; } public int size() { return size; } public void clear() { HashMap.this.clear(); } } However, since this is all private code, and we are past Tiger beta2 freeze, there is not sufficient urgency to do it now. You'll find a number of anachronisms hiding inside the implementation of collections due to recent language changes, and I hope we have the time to address them soon after Tiger. ###@###.###
|