JAVA: Why is my update of the ArrayList value of a specific key in a Hashmap changing all the other key values?

Question

I'd need your help because I have no idea why my update of a value (for a specific key) in a hashmap is changing almost all the other keys'value... Here's the corresponding code that you can try out:

import edu.duke.*;
import java.util.*;
import java.io.*;

public class WordsInFiles {
    private HashMap<String, ArrayList> map;

    public WordsInFiles(){
        map = new HashMap<String, ArrayList>();
    }

    private void addWordsFromFile(File f) {
        FileResource fr = new FileResource(f);
        String fileName = f.getName();
        ArrayList<String> newWord = new ArrayList<String>();
        for (String s : fr.words()){
            if (!map.containsKey(s)) {
                newWord.clear();
                newWord.add(fileName);
                map.put(s, newWord);
                System.out.println("New:" + s + "\t" + newWord);
            } else {
                ArrayList<String> currArr = map.get(s);
                if (!currArr.contains(fileName)){
                    currArr.add(fileName);
                    newWord = currArr;
                    System.out.println("Update:" +s + "\t" + newWord);
                    map.put(s, newWord);
                }
            }
            System.out.println(map + "\n");
        }
    }

    public void tester() {
        System.out.println("\n ****** New Tester Instance ****** ");
        buildWordFileMap();
    }
}

By running tester on 4 files: brief1.txt, brief2.txt, brief3.txt and brief4.txt that respectively contains the strings:

1. cats are funny and cute
2. dogs are silly
3. love animals cats and dogs
4. love birds and cats

I get:

 ****** New Tester Instance ****** 
New:cats    [brief1.txt]
{cats=[brief1.txt]}

New:are [brief1.txt]
{cats=[brief1.txt], are=[brief1.txt]}

New:funny   [brief1.txt]
{cats=[brief1.txt], are=[brief1.txt], funny=[brief1.txt]}

New:and [brief1.txt]
{cats=[brief1.txt], are=[brief1.txt], and=[brief1.txt], funny=[brief1.txt]}

New:cute    [brief1.txt]
{cats=[brief1.txt], are=[brief1.txt], and=[brief1.txt], cute=[brief1.txt], funny=[brief1.txt]}

New:dogs    [brief2.txt]
{cats=[brief1.txt], are=[brief1.txt], and=[brief1.txt], dogs=[brief2.txt], cute=[brief1.txt], funny=[brief1.txt]}

Update:are  [brief1.txt, brief2.txt]
{cats=[brief1.txt, brief2.txt], are=[brief1.txt, brief2.txt], and=[brief1.txt, brief2.txt], dogs=[brief2.txt], cute=[brief1.txt, brief2.txt], funny=[brief1.txt, brief2.txt]}

New:silly   [brief2.txt]
{cats=[brief2.txt], are=[brief2.txt], and=[brief2.txt], silly=[brief2.txt], dogs=[brief2.txt], cute=[brief2.txt], funny=[brief2.txt]}

New:love    [brief3.txt]
{love=[brief3.txt], cats=[brief2.txt], are=[brief2.txt], and=[brief2.txt], silly=[brief2.txt], dogs=[brief2.txt], cute=[brief2.txt], funny=[brief2.txt]}

New:animals [brief3.txt]
{love=[brief3.txt], cats=[brief2.txt], are=[brief2.txt], and=[brief2.txt], silly=[brief2.txt], dogs=[brief2.txt], animals=[brief3.txt], cute=[brief2.txt], funny=[brief2.txt]}

Update:cats [brief2.txt, brief3.txt]
{love=[brief3.txt], cats=[brief2.txt, brief3.txt], are=[brief2.txt, brief3.txt], and=[brief2.txt, brief3.txt], silly=[brief2.txt, brief3.txt], dogs=[brief2.txt], animals=[brief3.txt], cute=[brief2.txt, brief3.txt], funny=[brief2.txt, brief3.txt]}

{love=[brief3.txt], cats=[brief2.txt, brief3.txt], are=[brief2.txt, brief3.txt], and=[brief2.txt, brief3.txt], silly=[brief2.txt, brief3.txt], dogs=[brief2.txt], animals=[brief3.txt], cute=[brief2.txt, brief3.txt], funny=[brief2.txt, brief3.txt]}

Update:dogs [brief2.txt, brief3.txt]
{love=[brief3.txt], cats=[brief2.txt, brief3.txt], are=[brief2.txt, brief3.txt], and=[brief2.txt, brief3.txt], silly=[brief2.txt, brief3.txt], dogs=[brief2.txt, brief3.txt], animals=[brief3.txt], cute=[brief2.txt, brief3.txt], funny=[brief2.txt, brief3.txt]}

Update:love [brief3.txt, brief4.txt]
{love=[brief3.txt, brief4.txt], cats=[brief2.txt, brief3.txt], are=[brief2.txt, brief3.txt], and=[brief2.txt, brief3.txt], silly=[brief2.txt, brief3.txt], dogs=[brief2.txt, brief3.txt], animals=[brief3.txt, brief4.txt], cute=[brief2.txt, brief3.txt], funny=[brief2.txt, brief3.txt]}

New:birds   [brief4.txt]
{love=[brief4.txt], cats=[brief2.txt, brief3.txt], are=[brief2.txt, brief3.txt], and=[brief2.txt, brief3.txt], silly=[brief2.txt, brief3.txt], dogs=[brief2.txt, brief3.txt], animals=[brief4.txt], birds=[brief4.txt], cute=[brief2.txt, brief3.txt], funny=[brief2.txt, brief3.txt]}

Update:and  [brief2.txt, brief3.txt, brief4.txt]
{love=[brief4.txt], cats=[brief2.txt, brief3.txt, brief4.txt], are=[brief2.txt, brief3.txt, brief4.txt], and=[brief2.txt, brief3.txt, brief4.txt], silly=[brief2.txt, brief3.txt, brief4.txt], dogs=[brief2.txt, brief3.txt], animals=[brief4.txt], birds=[brief4.txt], cute=[brief2.txt, brief3.txt, brief4.txt], funny=[brief2.txt, brief3.txt, brief4.txt]}

{love=[brief4.txt], cats=[brief2.txt, brief3.txt, brief4.txt], are=[brief2.txt, brief3.txt, brief4.txt], and=[brief2.txt, brief3.txt, brief4.txt], silly=[brief2.txt, brief3.txt, brief4.txt], dogs=[brief2.txt, brief3.txt], animals=[brief4.txt], birds=[brief4.txt], cute=[brief2.txt, brief3.txt, brief4.txt], funny=[brief2.txt, brief3.txt, brief4.txt]}

And I really don't get why on the line "Update:are [brief1.txt, brief2.txt]" the code updates all the keys of map for this ArrayList value. And even though it would be updating all the keys'value: Why all the keys except for dogs?

Thanks in advance for your help!

Answer 1

1. Correct

The newWord you put in you map is always the same instance, so every keys have the same list as value, you change it and you see it everywhere, you need to put a new list each time :

for (String s : fr.words()){
    if (!map.containsKey(s)) {
        ArrayList<String> newWord = new ArrayList<>();         
        newWord.add(fileName);
        map.put(s, newWord);
    } else {
        ArrayList<String> currArr = map.get(s);
        if (!currArr.contains(fileName)){
            currArr.add(fileName);                            
        }
    }
}

---

2.Improve

• don't use raw types, use generics : HashMap<String, ArrayList<String>>, and when you create it you can simply use map = new HashMap<>

• use Set instead of List, it'll automaticlly avoid duplicate and you won't have to check if the filename is already present before add

• the first if does check & add, and the else does add, you may simplify in an check part then always add. And for the check part you can check and put at the same time with putIfAbsent() method from Map

---

3.Shorten

1. computeIfAbsent will try to get the value from the key and if there is not it'll insert it with a new HashSet.
2. the value if returned and ready to be used (the value is the HashSet)
3. just add the filename to this value, no need to check because we're using Set and no duplicate is possible

// map is now a HashMap<String, Set<String>>
for (String s : fr.words()){
    map.computeIfAbsent(s, k -> new HashSet<>()).add(filename);
}

Answer 2

You created the newWord once and then keep putting the same List to the map.

ArrayList<String> newWord = new ArrayList<String>(); 

Within the loop, you put it in as:

        if (!map.containsKey(s)) {
            newWord.clear();
            newWord.add(fileName);
            map.put(s, newWord); // your problem lies here
            // created just once but put in many times;
            System.out.println("New:" + s + "\t" + newWord);
        } 

You can replace that line with

map.put(s, new ArrayList<>());

Answer 3

It's because you keep putting the same ArrayList instance into the map.

After

map.put(s, newWord);

write:

newWord = new ArrayList<>();

Or, better, just declare newWord inside that block:

List<String> newWord = new ArrayList<>();
newWord.add(fileName);
map.put(s, newWord);

Or, in one line:

map.put(s, new ArrayList<>(Arrays.asList(fileName));

But I would suggest that your "lists" are actually Sets, since you are only adding them if they are not already present. If you want an order-preserving set, use LinkedHashSet as your value type.