CODEWITHSUNDEEP
javaarraylist
trs
trs

Reputation: 2454

removing duplicates from an arraylist

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

Upvotes: 0

Views: 2975

Answers (9)

scooby
scooby

Reputation: 11

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);

Upvotes: 1

GreenMatt
GreenMatt

Reputation: 18570

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.

Upvotes: 0

Paŭlo Ebermann
Paŭlo Ebermann

Reputation: 74750

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.

Upvotes: 0

Costis Aivalis
Costis Aivalis

Reputation: 13728

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++){

Upvotes: 3

nsfyn55
nsfyn55

Reputation: 15353

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

override equals and hashcode appropriatley

Upvotes: 2

JustinKSU
JustinKSU

Reputation: 4989

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

Upvotes: 4

RMT
RMT

Reputation: 7070

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.

Upvotes: 0

Omnaest
Omnaest

Reputation: 3096

custormers = new ArrayList(new HashSet(customers))

ensure the equals and hashmethod are correctly implemented

Upvotes: 1

talnicolas
talnicolas

Reputation: 14053

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.

Upvotes: 0

Related Questions