CODEWITHSUNDEEP
javadictionaryhashmap
Justin Ertley
Justin Ertley

Reputation: 93

Counting occurrences of a key in a Map in Java

I'm writing a project that captures Java keywords from a .java file and keeps track of the occurrences with a map. I've used a similar method in the past successfully, but I can't seem to adopt this method for my intended use here.

    Map<String,Integer> map = new TreeMap<String,Integer>();
    Set<String> keywordSet = new HashSet<String>(Arrays.asList(keywords));
    Scanner input = new Scanner(file);
    int counter = 0;
    while (input.hasNext())
    {
        String key = input.next();
        if (key.length() > 0)
        {
            if (keywordSet.contains(key))
            {
                map.put(key, 1);
                counter++;
            }

                if(map.containsKey(key)) <--tried inner loop here, failed
                {
                    int value = map.get(key);
                    value++;
                    map.put(key, value);
                }

        }

This block of code is supposed to add the keyword to the key, and increment the value each time the same key occurs. So far, it adds the keywords, but fails to properly increment the value. here is a sample output:

{assert=2, class=2, continue=2, default=2, else=2, ...}

Basically it increments every value in the map instead of the ones it's supposed to. I'm not sure if I'm over-thinking this or what. I've tried an inner loop and it gave me insane results. I really hope I'm just over-thinking this. Any help is greatly appreciated!

Upvotes: 6

Views: 35714

Answers (6)

Lars
Lars

Reputation: 2485

map.compute(key, (k, value) -> (value == null) ? 1 : (value + 1));

Upvotes: 1

David Soroko
David Soroko

Reputation: 9086

Even more concise using Map.merge (since Java 8):

if (keywordSet.contains(key)) {
    map.merge(key, 1, (currentCount, notUsed) -> ++currentCount);
}

Here is a generic implementation of a counting map - a map with values representing the count of their keys:

public static <K> void count(K key, Map<K, Integer> map) {
    map.merge(key, 1, (currentCount, notUsed) -> ++currentCount);
}

public static void main(String[] args) {
    Map<String, Integer> map = new HashMap<>();
    count("A", map);
    count("B", map);
    count("A", map);
    count("Z", map);
    count("A", map);
    System.out.println(map); // {A=3, B=1, Z=1}
}

Upvotes: 6

Bruno Reis
Bruno Reis

Reputation: 37822

There's a much more concise (and easier to reason about) way to achieve what you want:

final ConcurrentMap<String, AtomicInteger> map = new ConcurrentHashMap<>();
final Scanner input = new Scanner(file);
while (input.hasNext()) {
  final String key = input.next();
  if (key.length() > 0) {
    map.putIfAbsent(key, new AtomicInteger(0));
    map.get(key).incrementAndGet();
  }
}

Let's analyze why does this work.

Whenever the Scanner encounters a keyword, there are 2 possible cases: you either have encountered it before (ie, it is a known keyword), or it is an yet unseen keyword.

  • If it is an unseen keyword: putIfAbsent will put an AtomicInteger with value 0 in the map, and incrementAndGet() will set it to 1 right after, and, from now on, it becomes a known keyword;
  • If it is a known keyword: putIfAbsent will do nothing, and incrementAndGet() will increment the value that is already present in the map.

Then, if you want the key set, you do:

final Set<String> keys = map.keySet();

To print all the values, you could do something like:

for (final String k : map.keySet()) {
  System.out.println(k + ": " + map.get(k).get());
}

You are not forced to use the two "different" classes I used above, ConcurrentMap and AtomicInteger. It is just easier to use them because they encapsulate much of the logic that you tried to write by yourself (and failed). The logic that they encapsulate is exactly all the other answers describe (ie, test if the value is present, if not set it to 0, then get whatever value is present, increment it and put it back into the map).

To maintain the keys of the map (our words being counted) in alphabetical order, use a ConcurrentNavigableMap such as ConcurrentSkipListMap .

Upvotes: 13

Achintya Jha
Achintya Jha

Reputation: 12843

Map<String, Integer> map = new HashMap<String, Integer>();
Set<String> keywordSet = new HashSet<String>(Arrays.asList(keywords));
Scanner input = new Scanner(file);

while (input.hasNext()){
     String key = input.next();
     if (key.length() > 0)
        if (keywordSet.contains(key)){
           Integer counter = map.get(key);

           if (counter == null)
               map.put(key, 1);
           else
               map.put(key, count + 1);
         }
}

Upvotes: 1

Eyal Schneider
Eyal Schneider

Reputation: 22446

For every key you scan you create a new entry in the map (overriding the existing one). Then, the next condition holds so you increment the count by 1, reaching the value 2.

The inner part should be something like:

        if (keywordSet.contains(key))
        {
            Integer value = map.get(key);
            if (value == null)
                value = 0;
            value++;
            map.put(key, value);
        }

Anyway, consider using some kind of a mutable integer to make this more efficient. You won't have to override entries in the map, and you won't be doing too much Integer boxing operations.

Upvotes: 7

BobTheBuilder
BobTheBuilder

Reputation: 19284

You always set the value to 1 and then update it by another one. What you need is to update the map value (and not setting it to 1 again).

Instead of: 

map.put(key, 1);

use:

Integer value = map.get(key);
if (value == null){
    value = 0
}
value++;
map.put(key, value);

And drop the second if.

Upvotes: 2

Related Questions