current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
up vote 2 down vote favorite

This prints the character that occurs the most in a given string.

Input: String of word(s)

Output: Max occurring character

public class mostOccurringCharacter {
public static  Character findMaxOChar(String text){
    HashMap<Character, Integer> map = new HashMap<Character, Integer>();
    Character maxChar = ' ';
    long max = Integer.MIN_VALUE;
    for(int i=0;i<text.length();i++){
        Character current = text.charAt(i);
        if(map.containsKey(current)){
            map.put(current,map.get(current)+1);
        }
        else{
            map.put(current,1);
        }
        if(map.get(current)>max){
            maxChar = current;
            max = map.get(current);
        }
    }
    return maxChar;
}

public static void main(String args[]){
    String text = "wfaveqr caaaaaaaaaaaafwefwgqrvwerwbhqfvwrfwvbetqfwef q fwgwfw erfq";
    System.out.println("The max character is : "+ findMaxOChar(text));
}
}
share|improve this question
    
Could you please add a description of the code and what sort of input you are looking for? Refer to also How to Ask. – Tunaki Jun 25 at 21:59
    
Your algorithm could be faster if you used an array with each characters' ASCII value – Ishan M Jun 26 at 0:23
up vote 2 down vote

Primitives over wrapper classes

public static Character findMaxOChar(String text) { ... }

It's usually recommended to prefer the primitive type over its wrapper class, unless you have a clear intention to return null values for indicating the absence of a value.

Interfaces over implementations

HashMap<Character, Integer> map = new HashMap<Character, Integer>();

Using the Map interface over the HashMap implementation is often recommended, as subsequent usage of the object only needs to know it's a Map. Since Java 7, you can rely on type inference for generic object creation to eliminate the redundant type specification on the right, i.e.:

Map<Character, Integer> map = new HashMap<>();

Java 8 stream-based processing

Assuming you are on Java 8 and eager to learn about its new stream-based processing, this is a good candidate for adopting this approach. :) For starters, you need to understand:

  1. String.chars() can give you an IntStream of the string's characters, as long as we're talking about Western-based encoding.
  2. A Collector that lets you apply some processing on a Stream to return some form of results, e.g. a Map in your case. Converting an IntStream to Stream<Integer> by calling IntStream.boxed().

Building the Map can be just:

Map<Character, Long> map = text.chars()
                                .boxed()
                                .collect(Collectors.groupingBy(
                                            i -> Character.valueOf((char) i.intValue()),
                                            Collectors.counting()));

Here, we are groupingBy mapping the Integer values into Characters, and counting the number of occurrences.

Following the same idea, you can call Map.entrySet().stream(), i.e. to stream on map's entries, to:

  1. Group by the entries' values now, into a TreeMap, by mapping the entries' keys into a List,
  2. Get the TreeMap's last entry, and from that entry's value, find any value to return as the result.
share|improve this answer
up vote 1 down vote

mostOccurringCharacter doesn't make for a very good class name because it doesn't follow the PascalCase convention of styling. That said, it's a placeholder anyway.

Your code has a bug; empty string gives you "space" as most occurring character.

There is no need to keep track of maxChar and max because you would be better off finding the highest character after you've counted all the characters in the string.

Lastly, I don't know what the requirements are for when there are two most occuring characters. Like "aaabbb". You might need to return a list of characters.

share|improve this answer
    
Thanks for the review. The "There is no need to keep track of maxChar and max because you would be better off finding the highest character after you've counted all the characters in the string." I wouldn't want to go through the list again to find the max occurring character. Rather keep a track of it. Yes the space I agree would be a bad initialization. What do you suggest? And we are looking at 1 character that occurs the most, not multiple. – Neelesh Salian Jun 26 at 4:18
    
@NeeleshSalian "Going through the list" means the algorithm is 2n, but the question is "how fast is n now"? But what would you return in "aaabbb"? – Pimgd Jun 26 at 10:22
    
in the case of "aaabbb", since Max is updated when something is greater than it, it would be "a". To make sure we capture all max occurring, I am thinking a pass over the values in HashMap and returning the ones matching max should do the trick. Please correct me if I am wrong. – Neelesh Salian Jun 26 at 21:18

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.