tags:

views:

409

answers:

3

hi forum. I have a code to return an arrayList with the duplicates of an ArrayList but seems it{s not working, I am comparing all items in the array...

Sorry, I entered wrong code, I edited :)

public ArrayList<ObjectList> duplicates(ArrayList<ObjectList> someObjectsList) {

    ArrayList<ObjectList> ret = new ArrayList<ObjectList>();
    for ( ObjectList aSomeObjectsList: someObjectsList) {

        String field1 = aSomeObjectsList.get1();
        String field2 = aSomeObjectsList.get2();
        String field3 = aSomeObjectsList.get3();
        String field4 = aSomeObjectsList.get4();
        for (ObjectList someObject : ret) {
            if (
                field1.trim().equals(someObject.get1())&& 
                field2.trim().equals(someObject.get2())&&
                field3.trim().equals(someObject.get3())&&
                field4.trim().equals(someObject.get4())     
                ){
                ret.add(aSomeObjectsList);

            }
        }

    }
    return ret;
} 

But i guess I am doing something wrong because it doesnt return anything, and I know it has duplictates under this 4 filed criteria

Thankx in advance

+2  A: 
    for (Object someObject : ret) {
        if (
            field1.trim().equals(someObject.get1())&& 
            field2.trim().equals(someObject.get2())&&
            field3.trim().equals(someObject.get3())&&
            field4.trim().equals(someObject.get4())     
            ){
            ret.add(aSomeObjectsList);

        }
    }

The above loop wouldn't work, since it has the size of zero.

Here you go,

public Set<ObjectList> duplicates(ArrayList<ObjectList> someObjectsList) {

    Set<ObjectList> originals = new HashSet<ObjectList>();
    Set<ObjectList> duplicates = new HashSet<ObjectList>();

    for ( ObjectList aSomeObjectsList: someObjectsList) {
        boolean added = originals.add(aSomeObjectsList);
        if(!added){
            duplicates.add(aSomeObjectsList);
        }     
    }
    return duplicates;
} 

This would work, provided your ObjectList class have the correct implementation of hashCode() and equals() methods.

Disclaimer: This implementation will not provide the information about how many times a particular object was duplicated in the provided list. It will just tell you that a particular object was duplicated. I assumed that that was your real intention. If you wanna count, how many times, you have to modify the code accordingly.

Hint/Suggestion: You should override the equals() method and place your field equality check in there instead, once and for all.

Adeel Ansari
how do I provide the ObjectList class hashcode() and equals() I still dont get it.Do I have to create an interface? public class ObjectList implements Comparator<Object>{} etc...?????THANKS
Edgar
No, just override `equals()` and `hashcode()` method. No need to `extend` or `implement` anything. Look at the example here, http://www.javapractices.com/topic/TopicAction.do?Id=28 . And please try to learn and understand the language and its features. Grab a nice book or tutorial.
Adeel Ansari
+1  A: 

This shouldn't compile - if aSomeObjectsList is an Object then it doesn't have methods get1(), get2(), etc.

Your logic won't work because you aren't checking each element in your input List against the other elements in the input List; rather, you're trying to check the return List.

Also, this is not a really efficient way to check for duplicates in a collection. A better way would be to use a HashMap, where you could check set membership in roughly constant time. If you have to use a List, then sort it first (assuming your objects have a natural ordering) and check adjacent members for equality.

Barring those two, just use List.contains().

danben
True as well :). Didn't notice.
Adeel Ansari
+1  A: 

Here's a way you can do this. I have defined a basic class ObjectList that shows a way to implement equals and hashCode. Note that this assumes that all the internal variables are non-null. If these variables can contain null then you will need to check for that when computing the equals/hashCode. Also, the objects in this class must also themselves properly implement equals/hashCode.

public class ObjectList {

    private int h;

    private Object obj1;
    private Object obj2;
    private Object obj3;
    private Object obj4;

    @Override
    public boolean equals(final Object o) {
        if (!(o instanceof ObjectList))
            return false;

        final ObjectList that = (ObjectList) o;
        return that.obj1.equals(obj1) && that.obj2.equals(obj2)
            && that.obj3.equals(obj3) && that.obj4.equals(obj4);
    }

    @Override
    public int hashCode() {
        // caches the hashcode since it could be costly to recompute every time
        // but this assumes that your object is essentially immutable 
        // (which it should be if you are using equals/hashCode. If this is not
        // true and you want to just temporarily use this when doing the duplicate
        // test, move the h variable definition from the object level to this method
        // and remove this if statement.
        if (h != 0)
            return h;

        h = obj1.hashCode();
        h = h * 31 + obj2.hashCode();
        h = h * 31 + obj3.hashCode();
        h = h * 31 + obj4.hashCode();
        return h;
    }

}

public Collection<ObjectList> duplicates(
        final Collection<ObjectList> someObjectsList) {

    final Set<ObjectList> unique = new HashSet<ObjectList>(someObjectsList);
    final ArrayList<ObjectList> ret = new ArrayList<ObjectList>(someObjectsList);
    for (final ObjectList o : unique) {
        ret.remove(o);
    }

    // The ret list now contains the duplicate instances; instances 
    // with more than two occurrences will occur multiple times still in
    // this list.
    return ret;

    // If you want a list of unique duplicate instances then, comment out the above
    // return and uncomment this one.
    // return new HashSet<ObjectList>(ret);
}

Using Collection<ObjectList> is better, if you can do that, for both the parameter and returned value so you can vary the implementations (ArrayList, Set, etc).

Kevin Brock
Thanks I am doing this implementation and seems to work correctly, Also I know that I miss a lot of syntaxis of the language and a lot of its features, thanks Kevin
Edgar