removing duplicates from an arraylist

Question

I am trying to remove duplicate objects from an arraylist see code below:

ArrayList<Customer> customers=new ArrayList<Customer>();

    for(int i=0;i<accounts.size();i++){
        customers.add(accounts.get(i).getCustomer());
    }

    for(int i=0;i<customers.size();i++){
        for(int j=i+1;j<customers.size();j++){
            if(customers.get(i).getSocialSecurityNo().compareTo(customers.get(j).getSocialSecurityNo())==0){
                if(customers.get(i).getLastName().compareToIgnoreCase(customers.get(j).getLastName())==0){
                    if(customers.get(i).getFirstName().compareToIgnoreCase(customers.get(j).getFirstName())==0){
                        customers.remove(j);
                    }
                }
            }
    }
    }

However, it seems that the last object in the list is not being processed. Perhaps someone can pinpoint the error

possible duplicate - http://stackoverflow.com/questions/2435156/java-removing-duplicates-in-an-arraylist — mre, Jun 15 '11 at 18:08
If you follow tylermac's suggestions (which is a good one). Make sure to override the equals() and hashCode() methods. — JustinKSU, Jun 15 '11 at 18:25

score 4 · Accepted Answer · answered Jun 15 '11 at 18:18

4

Try adding j--; after removing an item. That will reindex for you and solve your issue.

answered Jun 15 '11 at 18:18

JustinKSU

4,875
2
29
51

Costis Aivalis · Answer 2 · 2011-06-15T18:26:23.343

3

The basic flaw is that since the ListArray is mutable, once you remove one element your indexes have to be readjusted.

if(customers.get(i).getFirstName().compareToIgnoreCase(customers.get(j).getFirstName())==0){
       customers.remove(j--);
}

also try subtracting one from your i loop:

for(int i=0;i<customers.size()-1;i++){
    for(int j=i+1;j<customers.size();j++){

edited Jun 15 '11 at 18:26

answered Jun 15 '11 at 18:15

Costis Aivalis

13,680
3
46
47

1

That's where iterators come into play. – Voo Jun 15 '11 at 18:26
Yeah, iterators are always better! – Costis Aivalis Jun 15 '11 at 18:28
2

@Voo: Iterators don't really help here, you will get ConcurrentModificationExceptions here on a stock ArrayList. – Paŭlo Ebermann Jun 15 '11 at 18:34
@Paulo Well you can get it actually work with an Iterator, but it doesn't look any better. So yeah not that great. – Voo Jun 15 '11 at 19:11

score 2 · Answer 3 · answered Jun 15 '11 at 18:19

2

    public static void removeDuplicates(ArrayList list) {
            HashSet set = new HashSet(list);
            list.clear();
            list.addAll(set);
    }

override equals and hashcode appropriatley

answered Jun 15 '11 at 18:19

nsfyn55

14,875
8
50
77

You should change the method signature to `removeDuplicates(Collection list)` (program to the interface and not a concrete class). – Steve Kuo Jun 15 '11 at 19:22
1

So I guess I can't use your method with a `LinkedList` or any other non-ArrayList collection. You'll just have to copy-paste the method and accept another class type. – Steve Kuo Jun 15 '11 at 19:47
`LinkedList` is one of the many collection in the Java collections framework. Take a look at http://download.oracle.com/javase/6/docs/api/java/util/Collection.html and look under "All Known Implementing Classes" for the various `Collection` implementations. – Steve Kuo Jun 15 '11 at 20:08
@Steve I'm not sure what that has to do with anything. – nsfyn55 Jun 15 '11 at 23:41
@nsfyn55 Java language specification are now driven by Oracle, so by using java, you are at their mercy... – Jun 21 '11 at 09:01

score 1 · Answer 4 · answered Jun 15 '11 at 18:15

1

custormers = new ArrayList(new HashSet(customers))

ensure the equals and hashmethod are correctly implemented

answered Jun 15 '11 at 18:15

Omnaest

3,096
1
19
18

score 1 · Answer 5 · answered Aug 23 '11 at 16:23

The code below worked for me. Give it a try. You can manipulate the compare method to suit your taste ArrayList customers = .....; Set customerlist = new TreeSet(new Comparator(){

        @Override
        public int compare(Customer c1, Customer c2) {
            return c1.getSocialSecurityNo().compareTo(c2.getSocialSecurityNo());
        }            
    });
    customerlist.addAll(customers);
    customers.clear();
    customers.addAll(customerlist);

score 0 · Answer 6 · answered Jun 15 '11 at 18:08

0

It's your int j=i+1 that causes trouble. You need to test with the last value of the customers list for each iteration.

answered Jun 15 '11 at 18:08

talnicolas

13,885
7
36
56

The j+1 just makes sure that each item is compared with itself. – JustinKSU Jun 15 '11 at 18:15

RMT · Answer 7 · 2011-06-15T18:18:22.400

0

Before you add them to the list in the above loop, why don't you check

if(!cutomers.contains(accounts.get(i).getCustomer())
{
//add them if it doesn't contain
}

It should save you from doing the second loop

Edit: Need to override the equals method.

edited Jun 15 '11 at 18:18

answered Jun 15 '11 at 18:08

RMT

7,040
4
25
37

@RMT, I once suggested this and was the focus of a lot of negativity. :) – mre Jun 15 '11 at 18:10
@mre that is true, its not the greatest thing to use, I think it should be better then having a 2nd and 3rd loop below.(just my opinion:) ) – RMT Jun 15 '11 at 18:14
1

There are a lot of better ways to do the problem, but that doesn't answer his/her question. What is wrong with his/her algorithm? – JustinKSU Jun 15 '11 at 18:15
@RMT I tried to do that and it doesn't seem to work for(int i=0;i – trs Jun 15 '11 at 18:16
@trs what i forget to metion you got to override the Equals method. basically a customer is equal when (from what i can tell) everything you have in your if statements. – RMT Jun 15 '11 at 18:18
@RMT how do you override the equals method? – trs Jun 15 '11 at 18:19
Ill post the link to the documentation in my answer – RMT Jun 15 '11 at 18:19
@trs let me know if you need anymore help – RMT Jun 15 '11 at 18:27

score 0 · Answer 8 · answered Jun 15 '11 at 18:33

So, about doing this right:

Your Customer objects should have an equals() and hashCode() method, which do the comparison. (Or you simply would have only one Customer object for each customer, which would mean your data model would have to be adjusted. Then the default hashCode/equals would do.)

If you have this, you can replace your three nested ifs with one:

    if(customers.get(i).equals(customers.get(j)) {
       customers.remove(j);
    }

This would not yet solve your problem, but make it easier to have a clearer look on it. If you look at which objects are compared to which others, you will see that after each removal of an object from the list, the next one has the same index as the one which you just removed, and you will not compare the current object to it. As said, j-- after the removal will solve this.

A more performant solution would be using a Set (which is guaranteed not to contain duplicates). In your case, a HashSet<Customer> or LinkedHashSet<Customer> (if you care about the order) will do fine.

Then your whole code comes down to this:

Set<Customer> customerSet = new HashSet<Customer>();

for(Account acc : accounts){
    customerSet.add(acc.getCustomer());
}
List<Customer> customers = new ArrayList<Customer>(customerSet);

If you don't really need a list (i.e. indexed access), ommit the last line and simply use the set instead.

score 0 · Answer 9 · answered Jun 15 '11 at 18:41

My first thought was to use Sets, as others have mentioned. Another approach would be to use Java's version of the foreach, instead of using indexes. A general approach:

public static ArrayList removeDuplicates(ArrayList origList) {
    ArrayList newList = new ArrayList();
    for (Object m : origList) {
        if (!newList.contains(m)) {
            newList.add(m);
        }
    }
    return newList;
}

In testing, I just used Strings; I'd recommend inserting Customer into the code where appropriate for type safety.

removing duplicates from an arraylist

9 Answers9