You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Mark Hindess (JIRA)" <ji...@apache.org> on 2008/04/24 16:17:22 UTC
[jira] Created: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
[classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
---------------------------------------------------------------------------------------
Key: HARMONY-5788
URL: https://issues.apache.org/jira/browse/HARMONY-5788
Project: Harmony
Issue Type: Bug
Components: Classlib
Reporter: Mark Hindess
The following code illustrates the problem:
import java.util.TreeMap;
public class TestTreeMap {
public static void main(String[] args) {
TreeMap master = new TreeMap<String, String>();
master.put("one", "1");
master.put("null", null);
TreeMap map = new TreeMap<String, String>();
Object[] element = master.entrySet().toArray();
for (int i = 0; i < element.length ; i++) {
System.out.println("Contains entry " + element[i] + "? == " +
map.entrySet().contains(element[i]));
}
}
}
On harmony, the output is:
Contains entry null=null? == true
Contains entry one=1? == false
on the RI, the output is:
Contains entry null=null? == false
Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Jimmy, Jing Lv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12593254#action_12593254 ]
Jimmy, Jing Lv commented on HARMONY-5788:
-----------------------------------------
Oops, sorry Mark I've made a mistake in the last post. It was trying to get a value, not an entry, so my fix is invalid.
I believe your fix is correct!
And IMHO, maybe we can refactor it a little as(as most entries will not have null value, this may improve performance as it avoid invoking containsKey(key) in most cases, which is costy):
public boolean contains(Object object) {
if (object instanceof Map.Entry) {
Map.Entry<K, V> entry = (Map.Entry<K, V>) object;
K key = entry.getKey();
Object v1 = TreeMap.this.get(key), v2 = entry.getValue();
return v1 == null ? TreeMap.this.containsKey(key) : v1.equals(v2);
}
return false;
}
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess closed HARMONY-5788.
---------------------------------
I've applied an additional fix (r652448) and test (r652449) for the TreeMap.subMap().
Closing now (and all of the commons-collections tests now pass).
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
> Assignee: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12593280#action_12593280 ]
Mark Hindess commented on HARMONY-5788:
---------------------------------------
I agree that refactoring might be helpful but unless I'm missing something then I think we still need to check that v2 == null, so something like:
return v1 == null ? ( v2 == null && TreeMap.this.containsKey(key) ) : v1.equals(v2);
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess resolved HARMONY-5788.
-----------------------------------
Resolution: Fixed
Assignee: Mark Hindess
I think we've eliminated all of the ways we can get this wrong. ;-) So I've committed a solution (and regressions tests for all the ways we got it wrong) in r652391.
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
> Assignee: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12592817#action_12592817 ]
Mark Hindess commented on HARMONY-5788:
---------------------------------------
Jimmy, The "v1 == null" test is effectively "TreeMap.this.get(entry.getKey()) == null" but this test does not allow us to distinguish the case where the entry is null from the case where the key does not exist. So I think we should leave the return line alone but those lines with a containsKey condition to rule out one of the two ambiguous cases. Like:
public boolean contains(Object object) {
if (object instanceof Map.Entry) {
Map.Entry<K, V> entry = (Map.Entry<K, V>) object;
K key = entry.getKey();
if (TreeMap.this.containsKey(key)) {
Object v1 = TreeMap.this.get(key),
v2 = entry.getValue();
return v1 == null ? v2 == null : v1.equals(v2);
}
}
return false;
}
Does this make sense? I'm going to run some more tests and wait for your comments before committing it.
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Jimmy, Jing Lv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12592304#action_12592304 ]
Jimmy, Jing Lv commented on HARMONY-5788:
-----------------------------------------
Hi Mark,
I believe this is a bug in this line 1206 :
if (object instanceof Map.Entry) {
Map.Entry<K, V> entry = (Map.Entry<K, V>) object;
Object v1 = TreeMap.this.get(entry.getKey()), v2 = entry.getValue();
>> return v1 == null ? v2 == null : v1.equals(v2);
this "v1" is a entry, not a key, so this line may be correct in Keyset.contains() but not in this EntrySet.contains()
As a result this line should be fixed to
return v1 == null ? false : v1.equals(v2);
If no objection, I'll apply this fix.
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5788) [classlib][util]
TreeMap.entrySet().contains() returns true for entries with null value
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12592491#action_12592491 ]
Mark Hindess commented on HARMONY-5788:
---------------------------------------
Jimmy, Thanks for looking at this but I think your solution breaks a containsAll test for entrySets of two identical maps. For example, append the following to the end of the above main method:
map.put("one", "1");
map.put("null", null);
System.out.println("containsAll? == " +
map.entrySet().containsAll(master.entrySet()));
On Harmony trunk without this change (and on RI) you get:
containsAll? == true
but on Harmony with the change you get:
containsAll? == false
> [classlib][util] TreeMap.entrySet().contains() returns true for entries with null value
> ---------------------------------------------------------------------------------------
>
> Key: HARMONY-5788
> URL: https://issues.apache.org/jira/browse/HARMONY-5788
> Project: Harmony
> Issue Type: Bug
> Components: Classlib
> Reporter: Mark Hindess
>
> The following code illustrates the problem:
> import java.util.TreeMap;
> public class TestTreeMap {
> public static void main(String[] args) {
> TreeMap master = new TreeMap<String, String>();
> master.put("one", "1");
> master.put("null", null);
> TreeMap map = new TreeMap<String, String>();
> Object[] element = master.entrySet().toArray();
> for (int i = 0; i < element.length ; i++) {
> System.out.println("Contains entry " + element[i] + "? == " +
> map.entrySet().contains(element[i]));
> }
> }
> }
> On harmony, the output is:
> Contains entry null=null? == true
> Contains entry one=1? == false
> on the RI, the output is:
> Contains entry null=null? == false
> Contains entry one=1? == false
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.