TreeSet contains method doesn't work for me

I want to put the custom's data into a TreeSet. When the custom number is same, I add the volume of trade.

Here is my TradeNode class that implements the Comparable Interator.

import java.util.Comparator;  

public class TradeNode implements Comparable<TradeNode> {  

    private String cstm; // custom number  

    private Integer mon = 0; // Trade  

    public TradeNode() {}  

    public TradeNode(String cstm, int mon) {  
        this.mon = mon;  
        this.cstm = cstm;  
    }  

    public int compareTo(TradeNode o) {  
        if (o.cstm.equals(this.cstm)) {  
            o.mon += this.mon;  
            return 0;  
        } else if (this.mon == o.mon) {  
            return this.cstm.compareTo(o.cstm);  
        } else {  
            //return (o.mon - this.mon);  
            return o.mon.compareTo(this.mon);  
        }  
    }  

    @Override  
    public boolean equals(Object obj) {  
        if (this == obj) {  
            return true;  
        }  
        if (obj == null) {  
            return false;  
        }  
        if (!(obj instanceof TradeNode)) {  
            return false;  
        }  
        TradeNode other = (TradeNode) obj;  
        if (cstm == null) {  
            if (other.cstm != null) {  
                return false;  
            }  
        } else if (!cstm.equals(other.cstm)) {  
            return false;  
        }  
        return true;  
    }  

    @Override  
    public int hashCode() {  
        final int prime = 31;  
        int result = 1;  
        result = prime * result + ((cstm == null) ? 0 : cstm.hashCode());  
        return result;  
    }  

    @Override  
    public String toString() {  
        return "[" + cstm + "] [" + mon + "]";  
    }  

    public int getMon() {  
        retu开发者_运维百科rn mon;  
    }  

    public void setMon(Integer mon) {  
        this.mon = mon;  
    }  

    public String getCstm() {  
        return cstm;  
    }  

} 

and the test class is :

public class Testtree {  
    public static void main(String[] args) {  
    TradeNode nd1 = new TradeNode("A", 100);  
        TradeNode nd2 = new TradeNode("B", 10);  
        TradeNode nd3 = new TradeNode("B", 1000);  
        TreeSet<TradeNode> tree = new TreeSet<TradeNode>();  
        tree.add(nd1);  
        tree.add(nd2);  
        tree.add(nd3);  
        for (TradeNode node : tree) {  
            System.out.println(node);  
        }  
    } 

I supposed the output shoud be like this :

[B] [1010]  
[A] [100]

but the output is

[B] [1000]  
[A] [100] 
[B] [10]

Could someone help me and point me out where my fault is?

if I change my compareTo() method like this, it still doesn't work.

public int compareTo(TradeNode o) {
        if (o.cstm.equals(this.cstm)) {
            return 0;
        } else {
            return o.mon.compareTo(this.mon);
        }
    }

and the result is:

[B] [1000]
[A] [100]
[B] [10]

I tryed Ben Xu's method, and here is the code: My new compareTo() method:

public int compareTo(TradeNode o) {
        if (o.cstm.equals(this.cstm)) {
            return 0;
        } else {
            return this.mon.compareTo(o.mon);
        }
    }

My new Testtree class:

public class Testtree {

    public static void main(String[] args) {
        TradeNode nd1 = new TradeNode("44010358010481", 150354);
        TradeNode nd2 = new TradeNode("44010358010481", 150641);
        TradeNode nd3 = new TradeNode("44010358010481", 270000);
        TradeNode nd4 = new TradeNode("44010039275685", 10000);
        TradeNode nd5 = new TradeNode("44010039275685", 980000);
        TradeNode nd6 = new TradeNode("44010039275685", 5000);
        TradeNode nd7 = new TradeNode("44010234235687", 10000);
        TradeNode nd8 = new TradeNode("44010234235687", 360000);
        TradeNode nd9 = new TradeNode("44010234235687", 53400);
        Map<String, Integer> map = new HashMap<String, Integer>(); 
        addTradeNode(map, nd1);
        addTradeNode(map, nd2);
        addTradeNode(map, nd3);
        addTradeNode(map, nd4);
        addTradeNode(map, nd5);
        addTradeNode(map, nd6);
        addTradeNode(map, nd7);
        addTradeNode(map, nd8);
        addTradeNode(map, nd9);

        Iterator<Entry<String, Integer>> iterator = map.entrySet().iterator();
        TradeNode t;
        List<TradeNode> list = new ArrayList<TradeNode>();
        while(iterator.hasNext()) {
            Map.Entry<String, Integer> m = iterator.next();
            t = new TradeNode(m.getKey(),m.getValue());
            list.add(t);
        }
        Collections.sort(list);
        for(TradeNode tn : list) {
            System.out.println(tn);
        }
    }

    private static void addTradeNode(Map<String, Integer> map, TradeNode node) {

        Integer integer = map.get(node.getCstm());
        if (integer == null) {
            map.put(node.getCstm(), node.getMon());
        } else {
            map.remove(node.getCstm());
            map.put(node.getCstm(), integer.intValue() + node.getMon());
        }

    }

}

and the result is:

[44010234235687] [423400]
[44010358010481] [570995]
[44010039275685] [995000]

finally, it satisfied my requirement. But I still don't know why this new compareTo() method doesn't work in the following test method:

public class Testtree2 {

    public static void main(String[] args) {
        TradeNode nd1 = new TradeNode("A", 100);
        TradeNode nd2 = new TradeNode("B", 10);
        TradeNode nd3 = new TradeNode("B", 1000);
        TreeSet<TradeNode> tree = new TreeSet<TradeNode>();
        tree.add(nd1);
        tree.add(nd2);
        tree.add(nd3);
        for (TradeNode node : tree) {
            System.out.println(node);
        }       
    }
}

and the result is:

[B] [10]
[A] [100]
[B] [1000]

and I supposed it to be :

[B] [10]
[A] [100]

could someone tell me where the fault is in my new compareTo() methods? Thanks a lot and thanks for anyone helping me.

Hahaha,I've got the answer from JavaRanch. There's someone named Henry told me the answer. Now I think when we use the contains() method in the TreeSet, it doesn't search everything in this Set, it only searched the sorted value.

The new Testtree3 class is :

public class Testtree3 {

    public static void main(String[] args) {
    TradeNode nd1 = new TradeNode("A", 100);
        TradeNode nd2 = new TradeNode("B", 200);
        TradeNode nd3 = new TradeNode("B", 1000);
        TreeSet<TradeNode> tree = new TreeSet<TradeNode>();
        tree.add(nd1);
        tree.add(nd2);
        tree.add(nd3);
        for (TradeNode node : tree) {
            System.out.println(node);
        }
    }

}

and the result is :

[A] [100]
[B] [200]

Haha. Now I'll go find the codes behinds the TreeSet.

---

TreeSet.add doesn't do what you think it does.

If it detects that a value already exists, it doesn't try to "add" the new value to the existing one - it just returns without changing the set. It's just a set-based operation.

(Additionally, the fact that your comparison is out of sync with your equals method is a little odd, and the comparison of this.mon == o.mon is inappropriate for Integer.)

---

The run result is , you can check to run the program again.

[B] [1000]
[A] [100]
[B] [10]

the result is due to treeset uses your implemented comparator

I don't know what you want to do.

but there is at least one obvious bad practice:

public int compareTo(TradeNode o) {  
    if (o.cstm.equals(this.cstm)) {  
        o.mon += this.mon;  
        return 0;  
    } else if (this.mon == o.mon) {  
        return this.cstm.compareTo(o.cstm);  
    } else {  
        //return (o.mon - this.mon);  
        return o.mon.compareTo(this.mon);  
    }  
}  

you should not change its value in a compareTo method “ o.mon += this.mon; ”, this is very confusing.

if you want to sum all TreeNode with the same name, don't use a Collection , use map instead.

for example, use a hashmap, key is name or(TreeNode since its equals and hashcode only uses cstm ), value is num. every time you add a TreeNode, check if the same name exsit, if not , add to map, else add the value.

following is one example code using map:

public class Testtree {
public static void main(String[] args) {
    TradeNode nd1 = new TradeNode("A", 100);
    TradeNode nd2 = new TradeNode("B", 10);
    TradeNode nd3 = new TradeNode("B", 1000);
    Map<String, Integer> map = new HashMap<String, Integer>();
    addTreeNode(map, nd1);
    addTreeNode(map, nd2);
    addTreeNode(map, nd3);
    System.out.println(map);
}

private static void addTreeNode(Map<String, Integer> map, TradeNode node) {

    Integer integer = map.get(node.getCstm());
    if (integer == null) {
        map.put(node.getCstm(), node.getMon());
    } else {
        map.remove(node.getCstm());
        map.put(node.getCstm(), integer.intValue() + node.getMon());
    }

}
}

---

You really shouldn't be mutating the argument in TradeNode#compareTo(...). There is no guarantee whether TreeSet will call newItem.compareTo(existingItem) or existingItem.compareTo(newItem) when doing comparisons.

You should probably fix your TradeNode#compareTo(...) so that it fulfills the Comparator contract without mutations.

I'm not sure that a Set (TreeSet or otherwise) is really the right data structure if you want to mutate the objects it contains. Perhaps a Map from String to TradeNode would be a better choice?

---

Your compareTo method contains code that changes state o.mon += this.mon;, which is very bad design, and worse, that state is used to determine the result of compareTo if (this.mon == o.mon). Since this toxic relationship exists, your implementation almost certainly violates the contract of compareTo: see its javadoc

This is horrible. Get rid of side-effect style state changes in your compareTo method.

public int compareTo(TradeNode o) {  
    if (o.cstm.equals(this.cstm)) {  
        o.mon += this.mon;    // ALARM BELLS!!! SIDE EFFECT!! ARRGGGHHH!
        return 0;  
    } else if (this.mon == o.mon) {  // AND THE SIDE EFFECT IS ALSO USED TO COMPARE! AVERT YOUR EYES! 
        return this.cstm.compareTo(o.cstm);  
    } else {  
        //return (o.mon - this.mon);  
        return o.mon.compareTo(this.mon);  
    }  
}