Skip to main content
5 of 5
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

Since you are on Java 8, this will be an ideal exercise for getting familiarized with the new stream-based processing approach. For example, instead of using an explicit for-loop:

for (int i = 0; i < s.length(); i++) {
    if (map.containsKey(s.charAt(i))) {
        // ...
    }
}

You can call String.chars() to give you an IntStream of 'int zero-extending the char values', or in other words a stream of characters.

Then, you only need to call Stream.collect() with a combination of Collectors.groupingBy() and Collectors.counting() to give you the resulting Map instance with the count-per-character:

Map<Character, Long> result = input.chars().boxed().collect(Collectors.groupingBy(
                                c -> Character.valueOf((char) c.intValue()),
                                Collectors.counting()));

For the first argument of Collectors.groupingBy(), we are mapping our Integer objects (which have been boxed() from the IntStream) to Character instances.

Again, instead of looping on result.entrySet(), we can apply another set of stream operations on it. First, we can define a Comparator that will compare an Entry object by the value:

private static final Comparator<Entry<? extends Object, Long>> RANK_BY_VALUE = Comparator
        .comparingLong(Entry::getValue);

Then, we will sort the stream of map entries with the reverse of this Comparator so that the characters with the most occurrences are ranked first:

Entry<Character, Long> entry = result.entrySet().stream()
                                .sorted(RANK_BY_VALUE.reversed()).findFirst().get();

Putting all these together into its own method:

private static Entry<Character, Long> getMaxOccurrenceCharacter(String input) {
    return input.chars().boxed().collect(
                    Collectors.groupingBy(
                            c -> Character.valueOf((char) c.intValue()),
                            Collectors.counting())).entrySet().stream()
            .sorted(RANK_BY_VALUE.reversed()).findFirst().get();
}

And here's an example of how you can make use of this method:

public static void main(String[] args) {
    Entry<Character, Long> result = getMaxOccurrenceCharacter("abc");
    System.out.printf("The character \"%s\" is repeated %d time(s).\n",
            result.getKey(), result.getValue());
}

// result
The character "a" is repeated 1 time(s).

Other pointers from your code:

  • Java's bracing convention is to put the opening brace on the same line, instead of what you have used (mostly). More importantly, have a consistent convention in your codebase. :) If you are more inclined to your current way, then do so throughout.

  • HashMap<Character, Integer> map should be declared using the Map interface, for simplification.

  • The method name getMaxViaHashmap() can be improved upon, as there should not be a need to tell callers how the derivation is done (ViaHashmap).

  • If your method is meant to be used solely for the derivation, it should not print output to System.out. From my example above, I get the results out first, then display it to the user.

  • Even if you want to do the equivalent of this manually:

      if (map.containsKey(key)) {
          map.put(key, map.get(key) + newValue);
      } else {
          map.put(key, newValue);
      }
    

    There are the newer Map.compute() or Map.merge() methods to help you. For example:

      // input being the String
      Map<Character, Long> result = new HashMap<>();
      for (int i = 0; i < input.length(); i++) {
          result.merge(Character.valueOf(input.charAt(i)), 1L, (a, b) -> a + b);
      }
    

edit: @Misha's answer is a welcome improvement to mine, do take a look at that too.

h.j.k.
  • 19.2k
  • 3
  • 36
  • 93