0

I am trying to count occurrence of each character in a string. So if I input aaabbbccc I am expecting o/p as {a=3, b=3, c=3} but instead I keep on getting it as {a=1, b=1, c=1} as my hashtable contains method fails and returns false. What is wrong in the code?

I also know that there is a HashMap collection quite similar to hashtable. but as I am learing java I want to try the same code with all datastructures just to get an idea of methods. The code with map is not much different than what I am doing here still this code fails. and I am not able to figure out where is the bug in the code.

I have following code:

Hashtable<Character, Integer> stringHash = new Hashtable<Character, Integer>();

This stringHash is a class level variable.

for(int i=0; i<s.length(); i++){
        if(stringHash ==null || stringHash.size()==0){
            stringHash.put(s.charAt(i), 1);
        }
        else{
            if(! stringHash.contains(s.charAt(i)) ){
                System.out.println(s.charAt(i));
                stringHash.put(s.charAt(i), 1);
            }
            else{
                int count = stringHash.get(s.charAt(i));
                stringHash.put(s.charAt(i), count++);
            }   
        }
        System.out.println(stringHash + " " + s.charAt(i) + "  "+ stringHash.contains(s.charAt(i)));
}
user1079065
  • 2,085
  • 9
  • 30
  • 53
  • 2
    I like your `if(stringHash ==null || stringHash.size()==0) stringHash.put(s.charAt(i), 1);` at the top there. That will immediately become a null reference exception. – Jashaszun Aug 18 '15 at 17:30
  • 2
    `if(stringHash ==null || stringHash.size()==0)` this is useless. – njzk2 Aug 18 '15 at 17:31
  • hastable is deprecated for your usage. see HashMap – njzk2 Aug 18 '15 at 17:32
  • Where does `count` come from? – PM 77-1 Aug 18 '15 at 17:34
  • ok will try with hashmap but my question is why is my contains failing? Though I am new to java and I am doing practice programs for collections, whether I use table or map, contains should not fail right? – user1079065 Aug 18 '15 at 17:36
  • @user1079065 ... check my answer below and upvote if that helps. There is an issue with the way you are using contains. – digidude Aug 18 '15 at 17:43
  • possible duplicate of [Creating an unordered map of in java](http://stackoverflow.com/questions/13595497/creating-an-unordered-map-of-char-int-in-java) – Tyler Durden Aug 18 '15 at 17:44

6 Answers6

4

This code works for me:

String s = "aaabbbccc";

Map<Character, Integer> stringHash = new HashMap<Character, Integer>();
for (char ch : s.toCharArray())
    stringHash.put(ch, stringHash.containsKey(ch) ? (stringHash.get(ch) + 1) : 1);

System.out.println(stringHash);
// output: "{a=3, b=3, c=3}"

I am using a Map<K, V> instead of HashTable<K, V>, but this is more common.

Jashaszun
  • 9,207
  • 3
  • 29
  • 57
2

Try something like this....The reason your code is failing is that you are checking contains() on HashTable instead of its keySet. Hope that helps

 public static void main(String[] args) {
    // TODO Auto-generated method stub
    String s = "aaaaabbcccc";
    Hashtable<Character, Integer> counter = new Hashtable<Character, Integer>();
    int count = 0;
    for(int i=0;i<s.length();i++){
        if(!counter.keySet().contains(s.charAt(i))){
            counter.put(s.charAt(i), 1);
        } else {
            count = counter.get(s.charAt(i));
            counter.put(s.charAt(i), ++count);
        }
    }

    for(char c:counter.keySet()) {
        System.out.println("Character : "+c+" - Occurences : "+counter.get(c));
    }
}

o/p

Character : b - Occurences : 2
Character : c - Occurences : 4
Character : a - Occurences : 5
digidude
  • 289
  • 1
  • 8
2

Your code

if(stringHash ==null || stringHash.size()==0){
    stringHash.put(s.charAt(i), 1);
}

would throw NPE if somehow the hashmap is null. Luckily it seems that you have initialized it properly. The block rather should have been

if(stringHash ==null){
    stringHash = new HashMap()
    stringHash.put(s.charAt(i), 1);
}

Again, that would not have fixed your bug. You should use containsKey instead of contains that checks for value in HashTable. What you are looking to implement can be summarized in following pseudocode.

initialize hashmap
for each character c in inputString
  count = 0
  if hashmap has a key for c
     count = get value for c from hashmap
  end if
  put in to hashmap c, count + 1
end for

In Java this would look like :

Map<Character, Integer> charCountMap = new HashMap<>();
for(char c : inputString.toCharArray()){
  int count = 0;
  if(charCountMap.containsKey(c)){
    count = charCountMap.get(c);
  }
  charCountMap.put(c,count+1);
}

Or for the adventurous, here is Java8 version

Map<Character,Long> map = s.chars().mapToObj(i->(char)i)
                                     .collect(Collectors
                                                .groupingBy(e -> e,
                                                   Collectors.counting()));
System.out.println(map);

Finally, do not use HashTable its a legacy class, no one uses it now a days. Stick with HashMap or other flavors of Map implementations.

Community
  • 1
  • 1
ring bearer
  • 20,383
  • 7
  • 59
  • 72
  • That's indeed a very elaborate answer and a good suggestiong about not using Hash Tables. But you would also want to include the fact here that the issue he was having was not actually becasue of using HashTable but because he was using the .contains() method wrong. Instead he sould have gone for .containsKey() or .keySet().contains() Upvoting :) – digidude Aug 18 '15 at 18:23
0

Debug my code questions are discouraged, but in the way of solving the general problem of counting characters in a string I can suggest a much simpler method:

public static int[] countCharacters( String s ){
    int[] count = new int[ 256 ];
    for( int xChar = 0; xChar < s.length(); xChar++ ) count[ s.charAt( xChar ) ]++;
    return count;
}   

This assumes you have single byte characters.

Tyler Durden
  • 11,156
  • 9
  • 64
  • 126
0

Why do you use hashMap for counting character occurance? I would use integer array of size 255 like so:

    int[] counter = new int[256];
    String s = "aaabbbcccc";
    for(int i = 0; i < s.length(); i++){
        counter[s.charAt(i)]++;
    }

    for(int i = 0; i < counter.length; i++)
        if(counter[i] > 0)
            System.out.println(((char)i) + " occurs " + counter[i] + " times");

that coude would give an output:

a occurs 3 times
b occurs 3 times
c occurs 4 times
Tawcharowsky
  • 615
  • 4
  • 18
0

Don't use Hashtable, you can simplify that code a lot, something like this should work:

import java.text.MessageFormat;
import java.util.HashMap;
import java.util.Map;

import org.apache.commons.lang.StringUtils;

public class Main {

    public static void main(String[] args) {
        Map<Character, Long> countMap = count("aabbbcccc");
        for (Map.Entry<Character, Long> entry : countMap.entrySet()) {
            System.out
                    .println(MessageFormat.format("Char ''{0}'' with count ''{1}''", entry.getKey(), entry.getValue()));
        }
    }

    private static Map<Character, Long> count(String value) {
        Map<Character, Long> countMap = new HashMap<Character, Long>();

        if (StringUtils.isNotBlank(value)) {
            for (int i = 0; i < value.length(); i++) {
                Long count = countMap.get(value.charAt(i));

                count = count == null ? 1 : count + 1;
                countMap.put(value.charAt(i), count);
            }
        }

        return countMap;
    }
}