HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Finding the most common character in a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thecharacterfindingstringcommonmost

Problem

I have started to try and work out the TopCoder problems. The "StringDup" problem asks us to:


Create a class called StringDup. Given a string made up of ONLY letters and
digits, determine which character is repeated the most in the string ('A' is
different than 'a'). If there is a tie, the character which appears first in
the string (from left to right) should be returned.

Is there any way I can make my solution better or more efficient? Is there something that I have missed? I'm new to Java and I thought this would be a good way to learn.

public class StringDup {

    public char getMax(String word)
    {
        int characterCount = 0;
        int maxCharacter = 0;
        char maxCharacterChar = '.';

        char[] cArray = word.toCharArray();

        for(int i =0; i  maxCharacter)
                    {
                        maxCharacter = characterCount;
                        maxCharacterChar = cArray[i];
                    }
                }
            }
        }
        return maxCharacterChar;
    }
}


It is called using the main method.

StringDup frequentChar = new StringDup();
System.out.print(frequentChar.getMax("sssdkljgh"));

Solution

Your algorithm has two loops, and that's really not necessary.

This question is looking for a special 'trick' to be done.... and that trick is to process the data in reverse order....

Consider this loop:

public static char getMax(String word) {
    if (word == null || word.isEmpty()) {
        throw new IllegalArgumentException("input word must have non-empty value.");
    }
    char maxchar = ' ';
    int maxcnt = 0;
    // if you are confident that your input will be only ascii, then this array can be size 128.
    int[] charcnt = new int[Character.MAX_VALUE + 1];
    for (int i = word.length() - 1; i >= 0; i--) {
        char ch = word.charAt(i);
        // increment this character's cnt and compare it to our max.
        if (++charcnt[ch] >= maxcnt) {
            maxcnt = charcnt[ch];
            maxchar = ch;
        }
    }
    return maxchar;
}


note the following things:

  • validate the input.



  • create an array of counters for each character.



  • work backwards... if this is the max count (or equal to it) then we have a new winner.



  • there is no validation on the input characters .... you should probably limit the chars to the a-zA-Z0-9 set that is specified.... the solution above will work for any String.



Some additional notes....

  • in Java, the standard code style puts the opening { brace on the same line as the block declaration (if condition, method declaration, etc.) Putting the brace on a new line is a C thing to do, and is frowned upon.



  • x is not a good variable name for anything unless it is a co-ordinate in a multidimensional array (normally linked with (x, y, ...) ). In your use case, you should use the variable j because that is the one that by convention is used inside the i loop.

Code Snippets

public static char getMax(String word) {
    if (word == null || word.isEmpty()) {
        throw new IllegalArgumentException("input word must have non-empty value.");
    }
    char maxchar = ' ';
    int maxcnt = 0;
    // if you are confident that your input will be only ascii, then this array can be size 128.
    int[] charcnt = new int[Character.MAX_VALUE + 1];
    for (int i = word.length() - 1; i >= 0; i--) {
        char ch = word.charAt(i);
        // increment this character's cnt and compare it to our max.
        if (++charcnt[ch] >= maxcnt) {
            maxcnt = charcnt[ch];
            maxchar = ch;
        }
    }
    return maxchar;
}

Context

StackExchange Code Review Q#40763, answer score: 7

Revisions (0)

No revisions yet.