Java, removing elements from an ArrayList

Question

I'm having an issue with this project. The basic premise is user enters a phrase and it's supposed to find any duplicate words and how many there are.

My issue is when entering just one word multiple times, such as... hello hello hello hello hello

The output for that would be;

"There are 2 duplicates of the word "hello" in the phrase you entered." 
"There are 1 duplicates of the word "hello" in the phrase you entered."

This only seems to happen in situations like this. If I enter in a random phrase with multiple words thrown in through out, it displays the correct answer. I think the problem has something to do with removing the duplicate words and how many times it iterates through the phrase, but I just cannot wrap my head around it. I've added print lines everywhere and have changed the times it iterates all sorts of ways, I through it in a Java Visualizer and still couldn't find the exact problem. Any help is greatly appreciated!

This is for an assignment for my online Java course, but it's only for learning/practice it does not go towards my major. I'm not looking for answers though just help.

public class DuplicateWords {

public static void main(String[] args) {

    List<String> inputList = new ArrayList<String>();
    List<String> finalList = new ArrayList<String>();

    int duplicateCounter;
    String duplicateStr = "";
    Scanner scan = new Scanner(System.in);

    System.out.println("Enter a sentence to determine duplicate words entered: ");
    String inputValue = scan.nextLine();
    inputValue = inputValue.toLowerCase();
    inputList = Arrays.asList(inputValue.split("\\s+"));
    finalList.addAll(inputList);


    for(int i = 0; i < inputList.size(); i++) {
        duplicateCounter = 0;
        for(int j = i + 1; j < finalList.size(); j++) {
            if(finalList.get(i).equalsIgnoreCase(finalList.get(j))
                    && !finalList.get(i).equals("!") && !finalList.get(i).equals(".")
                    && !finalList.get(i).equals(":") && !finalList.get(i).equals(";")
                    && !finalList.get(i).equals(",") && !finalList.get(i).equals("\"")
                    && !finalList.get(i).equals("?")) {
                duplicateCounter++;
                duplicateStr = finalList.get(i).toUpperCase();
            }
            if(finalList.get(i).equalsIgnoreCase(finalList.get(j))) {
                finalList.remove(j);
            }

        }
        if(duplicateCounter > 0) {
            System.out.printf("There are %s duplicates of the word \"%s\" in the phrase you entered.", duplicateCounter, duplicateStr);
            System.out.println();
        }
    }       
}
}

Based on some suggestions I edited my code, but I'm not sure I'm going in the right direction

String previous = "";

    for(Iterator<String> i = inputList.iterator(); i.hasNext();) {
        String current = i.next();
        duplicateCounter = 0;
        for(int j =  + 1; j < finalList.size(); j++) {
            if(current.equalsIgnoreCase(finalList.get(j))
                    && !current.equals("!") && !current.equals(".")
                    && !current.equals(":") && !current.equals(";")
                    && !current.equals(",") && !current.equals("\"")
                    && !current.equals("?")) {
                duplicateCounter++;
                duplicateStr = current.toUpperCase();
            }
            if(current.equals(previous)) {
                i.remove();
            }

        }
        if(duplicateCounter > 0) {
            System.out.printf("There are %s duplicates of the word \"%s\" in the phrase you entered.", duplicateCounter, duplicateStr);
            System.out.println();
        }
    }

You are iterating an ArrayList while removing items from it. This causes unexpected behaviour. A safe method to use is `Iterator.remove()`, see http://stackoverflow.com/a/223929/4190526 — Xiongbing Jin, Apr 04 '16 at 02:38
If you want to iterate an array while removing its items, I would recommend you start from the end of the array and iterate to the top. — annena, Apr 04 '16 at 02:43

score 1 · Answer 1 · answered Apr 04 '16 at 02:49

1

I would start by populating a Map<String, Integer> with each word; increment the Integer each time you encounter a word. Something like

String inputValue = scan.nextLine().toLowerCase();
String[] words = inputValue.split("\\s+");
Map<String, Integer> countMap = new HashMap<>();
for (String word : words) {
    Integer current = countMap.get(word);
    int v = (current == null) ? 1 : current + 1;
    countMap.put(word, v);
}

Then you can iterate the Map entrySet and display every key (word) where the count is greater than 1. Something like,

String msgFormat = "There are %d duplicates of the word \"%s\" in "
        + "the phrase you entered.%n";
for (Map.Entry<String, Integer> entry : countMap.entrySet()) {
    if (entry.getValue() > 1) {
        System.out.printf(msgFormat, entry.getValue(), entry.getKey());
    }
}

answered Apr 04 '16 at 02:49

Elliott Frisch

198,278
20
158
249

Wouldn't I have the same problem with it printing the result for each duplicate. Meaning "hello" would print say 4 duplicates and then the next pass through it would say there are 3 duplicates of "hello"....That's why I started removing the duplicates as I iterated through the list. – NoobCoderChick Apr 04 '16 at 03:28
@sjud9227 No. Because each word is a key, and thus **unique** in the `Map`. – Elliott Frisch Apr 04 '16 at 03:28
Oh ok. I don't understand a whole lot about Map and I have to turn this in tonight I may skip Map today and come back to it to practice. – NoobCoderChick Apr 04 '16 at 03:49

Bohemian · Accepted Answer · 2016-04-04T04:25:29.687

1

Your problem with your code is that when you remove an item, you still increment the index, so you skip over what would be the next item. In abbreviated form, your code is:

 for (int j = i + 1; j < finalList.size(); j++) {
     String next = finalList.get(i);
     if (some test on next)
         finalList.remove(next);
 }

after remove is called, the "next" item will be at the same index, because removing an item directly like this causes all items to the right to be shuffled 1 place left to fill the gap. To fix, you should add this line after removing:

 i--;

That would fix your problem, however, there's a cleaner way to do this:

 String previous = "";
 for (Iterator<String> i = inputList.iterator(); i.hasNext();) {
    String current = i.next();
    if (current.equals(previous)) {
        i.remove(); // removes current item
    }
    previous = current;
 }

inputList now has all adjacent duplicates removed.

To remove all duplicates:

List<String> finalList = inputList.stream().distinct().collect(Collectors.toList());

If you like pain, do it "manually":

 Set<String> duplicates = new HashSet<>(); // sets are unique
 for (Iterator<String> i = inputList.iterator(); i.hasNext();)
    if (!duplicates.add(i.next())) // add returns true if the set changed
        i.remove(); // removes current item

edited Apr 04 '16 at 04:25

answered Apr 04 '16 at 03:25

Bohemian

412,405
93
575
722

So, when using Iterator like you have above, you are assigning i to next and then I would use next in place of i, right? Would I be using an Iterator approach for the inner for loop as well? – NoobCoderChick Apr 04 '16 at 03:48
@sjud9227 I have edited the "proper" code to make it clearer and show you the entire thing. The key difference is that `iterator.remove()` doesn't change what `iterator.next()` returns. – Bohemian Apr 04 '16 at 03:52
I'm still a little confused, sorry I've been on my computer for 7 hours my brain is fried. So you're saying I wouldn't need the inner (j) for loop? – NoobCoderChick Apr 04 '16 at 03:57
@sjud9227 yes - assuming you only want to remove *adjacent* duplicates, you don't need the inner loop. So "hello hello hello world hello" would become "hello world hello". – Bohemian Apr 04 '16 at 04:02
I want to remove all duplicates after the count is made the first iteration so it does not repeat the answer over and over. Example. "hello hello hello world hello world" Would have an answer of "There are 3 duplicates of the word 'Hello'" and also "There are 1 duplicates of the word 'World'" I was removing the duplicates so it would not continue to count duplicates each pass. – NoobCoderChick Apr 04 '16 at 04:08
Does that make sense? – NoobCoderChick Apr 04 '16 at 04:08
To remove all duplicates, it's just one line: `List finalList = inputList.stream().distinct().collect(Collectors.toList());` – Bohemian Apr 04 '16 at 04:20
I updated my code up top, can you take a look and give me a tip on what to do next...Your suggestion works, but it's iterating and printing multiple times. Which makes sense but I can't figure out how to make it work without the two loops and comparing the two ArrayLists with a one of index – NoobCoderChick Apr 04 '16 at 04:22
@sjud9227 see edits for all variations you should need – Bohemian Apr 04 '16 at 04:25

score 0 · Answer 3 · answered Apr 04 '16 at 02:39

0

Before you add inputList to finalList, remove any duplicate items from inputList.

answered Apr 04 '16 at 02:39

Zamrony P. Juhara

5,222
2
24
40

I can't remove anything from the inputList because I used Arrays.asList to split the user input and I guess the List is not modifiable at that point. – NoobCoderChick Apr 04 '16 at 03:25

Java, removing elements from an ArrayList

3 Answers3