Count occurrences of a String in a String[] by a Map<String, AtomicInteger>

Question

The following method is supposed to count the occurrences of each item in the given set:

void groupBy(String[] stuff)  {
LinkedHashMap<String, AtomicInteger> A = new LinkedHashMap<String, AtomicInteger>();

final AtomicInteger one = new AtomicInteger(1);
AtomicInteger count;

for (String key:stuff)  {
    count = A.get(key);
    if (count==null) A.put(key, one);
        else System.out.println("Previous value  :"+A.put(key, new AtomicInteger(count.incrementAndGet())));
}

  Set set = A.entrySet();
  Iterator ii = set.iterator();

  while(ii.hasNext()) {
     Map.Entry me = (Map.Entry)ii.next();
     System.out.print(me.getKey() + ": ");
     System.out.println(me.getValue());
  }
}

So, if I run it on param

String a[] = {"AAA", "A", "AA", "B", "A", "AAA"};

i'm supposed to get

Previous value :1
Previous value :1
AAA: 2
A: 2
AA: 1
B: 1

But, what i'm getting is

Previous value :2
Previous value :3
AAA: 3
A: 2
AA: 3
B: 3

the values in the hash are updated beyond what i intend to do and i havent an idea how.

help appreciated.

Answer 1

You have problem at A.put(key, one); change it to A.put(key, new AtomicInteger(1)); and this will work

also, there is really no sense in AtomicInteger, you can rewrite code as:

LinkedHashMap<String, Long> A = new LinkedHashMap<String, Long>(stuff.length);
for (String key: stuff)
    A.put(key, (A.containsKey(key)?A.get(key):0L)+1L);

Answer 2

You appear to be accidentally reusing the same AtomicInteger value with different keys. When you place an AtomicInteger into the map, it can be re-used when you call get.

Here's what's going on:

Input: AAA

It doesn't exist yet, so one is placed in the map.

Input: A

It doesn't exist yet, so one is placed in the map. There are now two references to one in the map.

Input: AA

It doesn't exist yet, so one is placed in the map. There are now three references to one in the map.

Input: B

It doesn't exist yet, so one is placed in the map. There are now four references to one in the map.

Input: A

A already exists, so the value is retrieved. Now count refers to the same object as one! count is incremented, but a copy is made. Now AAA, AA, and B are still mapped to the original AtomicInteger, but it's now wrong; it's 2. However, A refers to a second AtomicInteger, which is correct at 2.

Input: AAA

AAA already exists, so the value is retrieved. Now count refers to the same object as one, again! count is incremented, but a copy is made. Now, AA and B are still mapped to the original AtomicInteger, but it's now wrong; it's 3. However, A refers to the second AtomicInteger still, and it's still correct at 2. Additionally, AAA refers to a third AtomicInteger, but it's still wrong at 3.

Solution

Change when you create new AtomicInteger objects, and just increment when you don't need new ones; they're mutable.

for (String key : stuff)  {
    AtomicInteger count = A.get(key);
    if (count == null)
        A.put(key, new AtomicInteger(1));
    else
        count.incrementAndGet();  // Modifies the object referred to in the map.
}