HashMap - contains and get methods should not be used together

Question

I got the following question from an interview.

I was given a character array like this:

char[] characters = {'u', 'a', 'u', 'i', 'o', 'f', 'u'}; 

I needed to get the distinct characters and counts of each character:

u = 3 a = 1 i = 1 o = 1 f = 1 

So I answered in Java with the following code:

HashMap<Character, Integer> map = new HashMap<Character, Integer>(); int i = 1; for (char c : characters) {                  if (map.containsKey(c)) {         int val = map.get(c);         map.put(c, ++val);     } else map.put(c, i); } 

The interviewer was a solution architect. He asked me why I was using both containsKey() and get() methods here and noted that it was redundant to use both methods. What is his point? What was I doing wrong here? Will my code cause a performance issue, etc.?

Answer 1

The architect means that get and containsKey have the same costs and may be accumulated into one check:

Integer val = map.get(c); if (val != null) {   ... } else {   ... } 

But I wonder why the architect is only concerned about that, as there are more things to improve:

• Refer to objects by their interfaces (Effective Java 2nd Edition, Item 52)
• Since Java 1.7 you can use the diamond operator <>
• Accumulate the autoboxing operations of the characters
• If you use AtomicInteger (or any other modifiable number class) instead of Integer you can even merge the get with one of the puts

So from my point of view the best performance, when using a HashMap, would offer:

Map<Character, AtomicInteger> map = new HashMap<>(); for (Character c : characters) {     AtomicInteger val = map.get(c);     if (val != null) {         val.incrementAndGet();     } else {         map.put(c, new AtomicInteger(1));     } } 

If the range of your characters is small (and known in advance), you could use an int array for counting. This would be the fastest of all possible solutions:

char firstCharacter = 'a'; char lastCharacter = 'z'; int[] frequency = new int[lastCharacter - firstCharacter + 1]; for (char c : characters) {   frequency[c - firstCharacter]++; } 

Answer 2

Your code is redundant, since both get and containsKey do pretty much the same work. Instead of calling containsKey you can check whether get returns a null value.

The code can be reduced to :

HashMap<Character, Integer> map = new HashMap<Character, Integer>(); for (char c : characters) {        Integer val = map.get(c);               if (val == null)         val = 0;     map.put(c,++val); }