0 
public static ArrayList<Character> removeDuplicates (ArrayList<Character> data) {
    ArrayList<Character> newList = new ArrayList<Character>();

    for (int i = 0; i < data.size() - 1; i++) {
      if (!newList.contains(data.get(i))) 
         newList.add(0,(data.get(i)));
    }
    return newList;
  }

Here is my code so far. I'm not understanding how this is not working

Improve this question
6
  • This is a very inefficient implementation, but it should work Commented Sep 10, 2015 at 20:47
  • 1
    I think you're always adding the the number to index 0, so it will just overwrite 0. Just use add(data.get(i)) to add it to the end of the list Commented Sep 10, 2015 at 20:49
  • 1
    @jackie wrong, he uses add not set Commented Sep 10, 2015 at 20:54
  • 1
    @Dici thanks for the correction. I should have read the documentation further. "Inserts the specified element at the specified position in this list. Shifts the element currently at that position (if any) and any subsequent elements to the right (adds one to their indices)." Commented Sep 10, 2015 at 20:58
  • 1
    The question is, what do you mean by "not working"? Commented Sep 10, 2015 at 21:00

2 Answers 2

Reset to default
2

You can use a Set. A Set is a Collection that doesn't let duplicates of Objects.

I'm not sure what Object type your List is of, but let's say your were using String:

//replace all instances of 'String' with whatever Object type you are using
Set<String> mySet = new HashSet<>();
for(String s : data){ 
  mySet.add(s);
}

Then if you want to send the data to a List, do:

ArrayList newList = new ArrayList(mySet);
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Using a set is good, but you don't need to populate it manually. You could just do new ArrayList<>(new HashSet<>(data)), but I guess this breaks the problem too easily
1

There are 2 problems with your implementation.

  1. You're not counting all of the items in the array. You should do either i <= data.size() - 1 or i < data.size(). Right now you're missing the last item.
  2. You're not adding items to the end of the list. Instead, you're repeatedly overwriting the zeroth (first) value. EDIT: Sorry, that was incorrect. You're inserting at the beginning of the list, which will work but is inefficient for the most commonly used lists (e.g. ArrayList).

Here is the fixed version. The problem areas are commented out with /* */.

List<Object> newList = new ArrayList<Object>();

for (int i = 0; i < data.size() /* - 1 */ ; i++) {
    if (!newList.contains(data.get(i)))
        newList.add( /* 0, */ (data.get(i)));
}

return newList;

EDIT: Using contains(...) on a list is slow. You can optimize and simplify this by using a Set. A set is a collection which has unique values. Adding the same value twice has no effect. We can take it a step further and use a LinkedHashSet as the implementation class, which will maintain the same ordering as was in the original list.

return new ArrayList<Object>(new LinkedHashSet<Object>(data));
Improve this answer

4 Comments

As @Dici points out, #2 is incorrect. See javadocs: public void add(int index, E element) Inserts the specified element at the specified position in this list. Shifts the element currently at that position (if any) and any subsequent elements to the right (adds one to their indices).
@jackie Thanks, you're right. I updated my answer.
Actually the real good solution is using a Set, but I +1 the answer since it fixes the OP's code
@Dici Yes, you're absolutely right. I added an improved solution that uses a LinkedHashSet, which will also maintain the original list ordering.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.