How to remove Duplicate objects from Java ArrayList?

Question

I need your help with some sample code for a situation I could not get free from. I have a simple list of objects. My class is like this:

class MyClass {
    String str;
    Integer intgr;
}

And the list contains elements like:

[{a1  5}, {b2  3}, {g1  1}, {b5  1}, {c9  11}, {g2  3}, {d1  4}, {b3  19}... ... ...]

I need to check if any element contain the same prefix in string (here suffix is the last single character) then keep that element which have greater value in integer. The expected output from the above example list will be:

[{a1  5}, {c9  11}, {g2  3}, {d1  4}, {b3  19}... ... ...]

Strings will have unique values but could have matches in prefix. I'm not that good in java. So can anybody help me out from this? Here is the code I'm trying but getting IndexOutOfBoundsException. This code has faults, so need some help from you.

Thanks!

        int size = list.size();
        for (int j = 0; j < size; j++) {
        if (list.get(j).str.substring(0, list.get(j).str.length()-1).compareTo(list.get(j+1).str.substring(0, list.get(j+1).str.length()-1)) == 0) {
            if (list.get(j).intgr > list.get(j+1).intgr)
                list.remove(list.get(j+1));
                size--;
            else {
                list.remove(list.get(j));
                j--;
                size--;
            }
        }
    }

One suggestion I'll give you: instead of chaining methods like that, try only doing one a time and read them into temporary variables. It will be far easier to see where your program's logic is wrong. — Charles
– Charles, Commented Jun 27, 2012 at 18:41

Francisco Spaeth · Accepted Answer · 2012-06-27 19:09:08Z

You could iterate over your collection of elements adding them into a Map associating the key (prefix) to the value (object). Every time an element is added you check if the element stored with the same prefix is bigger than the one being added.

In order to have this behavior:

provides this: [{a1 5}, {b2 3}, {g1 1}, {b5 1}, {c9 11}, {g2 3}, {d1 4}, {b3 19}]
results this: [{a1 5}, {c9 11}, {g2 3}, {d1 4}, {b3 19}]

You could implement something like this:

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

public class TestSystemOut {

    public static void main(final String[] a) {
        List<MyObj> list = prepareList();
        System.out.println("provides this: " + list);

        Map<String, MyObj> map = new LinkedHashMap<String, MyObj>(); // if result order doesn't matter this can be a simple HashMap

        String strTmp;
        for (MyObj obj : list) {

            strTmp = obj.str;
            strTmp = strTmp.substring(0, strTmp.length() - 1);

            if (map.get(strTmp) == null || map.get(strTmp).integer < obj.integer) {
                map.remove(strTmp); // this could be removed if order of result doesn't matter
                map.put(strTmp, obj);
            }
        }

        list.clear();
        list.addAll(map.values());

        System.out.println("results this: " + list);
    }

    public static class MyObj {
        String str;
        Integer integer;

        public MyObj(final String str, final Integer integer) {
            super();
            this.str = str;
            this.integer = integer;
        }

        @Override
        public String toString() {
            return "{" + str + " " + integer + "}";
        }

    }

    private static List<MyObj> prepareList() {
        List<MyObj> list = new ArrayList<MyObj>();
        list.add(new MyObj("a1", 5));
        list.add(new MyObj("b2", 3));
        list.add(new MyObj("g1", 1));
        list.add(new MyObj("b5", 1));
        list.add(new MyObj("c9", 11));
        list.add(new MyObj("g2", 3));
        list.add(new MyObj("d1", 4));
        list.add(new MyObj("b3", 19));
        return list;
    }
}

Ted Hopp · Accepted Answer · 2012-06-27 21:31:33Z

0

There are two problems with your code. First, when j == size - 1 (the last iteration), you are calling list.get(j+1), which is what is causing the exception. Just change your loop condition to j < size - 1 and the exception should go away. (Alternatively, start at j = 1 and compare to the previous element.)

Second, you are only comparing each element with its immediate successor element. From your description, it doesn't sound like that's what you want to do.

I'd suggest capturing the logic of comparison in a separate method. It might be part of MyClass:

class MyClass {
    String str;
    Integer intgr;
    /**
     * Returns the relationship between this and another MyClass instance.
     * The relationship can be one of three values:
     * <pre>
     *   -1 - This object should be discarded and other kept
     *    0 - There is no relationship between this and other
     *    1 - This object should be kept and other discarded
     * </pre>
     *
     * @param other The other instance to evaluate
     *
     * @return 0 if there is no relationship.
     */
    public int relationTo(MyClass other) {
        final String myPrefix = str.substring(0, str.length() - 1);
        final String otherPrefix = other.str.substring(0, other.str.length() - 1);
        if (myPrefix.equals(otherPrefix)) {
            return intgr < other.intgr ? -1 : 1;
        } else {
            return 0;
        }
    }
}

(This can easily be transformed into a method with two arguments that would be outside MyClass.) Then you can use the method to decide what to keep. You need to do a double iteration to find non-adjacent objects:

int size = list.size();
for (int i = 0; i < size; ++i) {
    final MyClass current = list.get(i);
    for (int j = 0; j < i; ++j) {
        final MyClass previous = list.get(j);
        final int relation = previous.relationTo(current);
        if (relation < 0) {
            // remove previous (at index j)
            list.remove(j);
            --i;
            --j;
            --size;
        } else if (relation > 0) {
            // remove current (at index i)
            list.remove(i);
            --i;
            --size;
            break; // exit inner loop
        }
        // else current and previous don't share a prefix
    }
}

edited Jun 27, 2012 at 21:31

answered Jun 27, 2012 at 19:27

Ted Hopp

235k48 gold badges412 silver badges533 bronze badges

2 Comments

divine Over a year ago

thanks for your answer! but wonder why I'm getting the one having samller value in intgr from the matching strings rather than the larger one that I need!

Ted Hopp Over a year ago

@divine - It's because I screwed up the logic in my last code block of what to remove based on the relationship between current and previous. I updated my answer to something that (I hope) now works.

assylias · Accepted Answer · 2012-06-27 18:55:32Z

Another (possibly easier to read & debug) way would be to:

put your items in a List
iterate over the list and populate a Map where String is the letter and MyClass is the minimum (i.e. if the map already contains something for "a", check if your new MyClass is bigger or smaller and replace accordingly)

Simple implementation:

public class Test {

    public static void main(String[] args) throws InterruptedException {
        List<MyClass> list = Arrays.asList(new MyClass("a1", 5),
                new MyClass("b2",  3),
                new MyClass("g1",  1),
                new MyClass("b5",  1),
                new MyClass("c9",  11),
                new MyClass("g2",  3),
                new MyClass("d1",  4),
                new MyClass("b3",  19));

        Map<String, MyClass> map = new HashMap<String, MyClass>();

        for (MyClass mc : list) {
            MyClass current = map.get(mc.getLetter());
            if (current == null || mc.intgr > current.intgr) {
                map.put(mc.getLetter(), mc);
            }
        }
        System.out.println(map);
    }

    static class MyClass  {

        String str;
        Integer intgr;

        MyClass(String str, Integer intgr) {
            this.str = str;
            this.intgr = intgr;
        }
        String getLetter() {
            return str.substring(0,1);
        }

        @Override
        public String toString() {
            return "[" + str + " " + intgr + "]";
        }
    }
}

Rossiar · Accepted Answer · 2012-06-27 19:01:49Z

/*
 * For every string in the list, look at all strings in the
 * list and compare the substring from 0 to length-1, if they
 * match and the id is not the same as the current s (i.e. s and
 * list.get(i) are at the same address) then remove that string.
 */
public ArrayList remdup(ArrayList<String> list) {

    for (String s : list) {
        for (int i=0; i<list.size();i++) {
            if (s.substring(0, s.length()-1).compareTo(list.get(i).substring(0, list.get(i).length()-1)) == 0
                    && list.indexOf(list.get(i)) != list.indexOf(s)) {
                list.remove(list.get(i));
            }
        }
    }
    return list;
}

Maybe give this a try, haven't tested yet but figure it should work. If it doesn't maybe try using something different to compareTo(), maybe use equals() as an alternative.

Also, you are getting an IndexOutOfBoundsException because you are subtracting off of size every time you remove an element, making your search area smaller. You have not taken into consideration that remove() does this for you.

I would consider using a Map for this, instead of a class. That way you don't have to go about re-inventing the wheel.

Collectives™ on Stack Overflow

How to remove Duplicate objects from Java ArrayList?

4 Answers 4

Comments

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related