David Martin
David Martin

Reputation: 95

Java duplicate catcher for loop still returning dupes

I have a method that is intended to remove duplicate objects from an ArrayList. The objects are of a custom class, IndividualEntry and the method looks like this:

 private static ArrayList<IndividualEntry> cleanList(ArrayList<IndividualEntry> inputList){
        ArrayList<IndividualEntry> thisList = inputList;
        IndividualEntry thisEntry;
        IndividualEntry thatEntry;
        for(int i = 0; i<thisList.size();i++){
            thisEntry = thisList.get(i);
            System.out.println("First entry is "+thisEntry);
        for(int j = (i+1); j<thisList.size(); j++){
            thatEntry = thisList.get(j);
            System.out.println("Second entry is "+thatEntry);
            if(thisEntry.equals(thatEntry)){
                thisList.remove(thatEntry);
            System.out.println("Entry removed: "+thatEntry);
            }
        }
            }
        return thisList;
    }

The method is successfully removing SOME duplicates. The ArrayList before this method runs looks like this (each letter representing a unique object):

A B B C A B B A A B B B B C

After I run the method it comes out as:

C A B B B C

I don't understand why this method would be rearranging the results and still including duplicates, but my suspicion is that it's due to the thisList getting changed within the inner for loop but the overall for loop still using the original value of thisList. Is that correct? What would be the fix for this?

Upvotes: 1

Views: 58

Answers (2)

GBlodgett
GBlodgett

Reputation: 12819

Using java 8+ you can do the following to remove the duplicates:

private static List<String> cleanList(List<String> inputList){
    return inputList.stream().distinct().collect(Collectors.toList());
}

This will create a stream of the List, only accept unique values, and then collect them to a List

Upvotes: 3

Mureinik
Mureinik

Reputation: 311188

by removing an item from a list while iterating over it, you're screwing up your logic. You don't get an obvious explicit failure like you'd have gotten with iterators, but sine the loops don't take in to consideration the removed items, you're just missing items.

A much easier approach would be to utilize the JDK's LinkedHashSet that both guarantees a single instance of each value and preserves the order of insertion:

private static ist<IndividualEntry> cleanList(List<IndividualEntry> inputList) {
    return new ArrayList<>(new LinkedHashSet<>(inputList));
}

This of course assumes that your IndividualEntry class properly implements the equals(Object) and hashCode() methods.

Upvotes: 3

Related Questions