2
\$\begingroup\$

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));
}
}
\$\endgroup\$
  • \$\begingroup\$ Could you please add a description of the code and what sort of input you are looking for? Refer to also How to Ask. \$\endgroup\$ – Tunaki Jun 25 '16 at 21:59
  • \$\begingroup\$ Your algorithm could be faster if you used an array with each characters' ASCII value \$\endgroup\$ – Ishaan Jun 26 '16 at 0:23
2
\$\begingroup\$

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.
\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – Neelesh Salian Jun 26 '16 at 4:18
  • \$\begingroup\$ @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"? \$\endgroup\$ – Pimgd Jun 26 '16 at 10:22
  • \$\begingroup\$ 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. \$\endgroup\$ – Neelesh Salian Jun 26 '16 at 21:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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