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

Counting the most frequently appearing character in a string (i.e. the statistical mode)

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

Problem

This is my sample code to find maximum occurrence of characters in string in C#.

How can I make it better?

public static int GetMaximumOccurence(string str)
{
    int _maxcount = 0;

    foreach (char c in str)
    {
        int ix = str.IndexOf(c);
        int count = 0;
        while (ix!=-1)
        {
            StringBuilder _s = new StringBuilder();
            _s.Append(str);
            _s[ix] = ' ';
            str = _s.ToString();

            count++;

            ix = str.IndexOf(c);
        }

        if (count > _maxcount)
        {
            _maxcount = count;
        }
    }

    return _maxcount;
}

Solution

I'm worried this is turning into Code Golf, so I'll preface my alternative with some actual review.

There is a pretty big bug in your code, as the following will not terminate:

Console.WriteLine(GetMaximumOccurence(" "));


Can you see why?

The basic structure of your code is good. That is, the part that effectively goes

var maxCount = 0;
foreach (var c in input)
{
    // TODO: Count occurrences of c in input.
    maxCount = Math.Max(count, maxCount);
}

return maxCount;


Now how to we complete that TODO? Using IndexOf like you do seems like a good idea.

var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    var i = input.IndexOf(c);
    while (i != -1)
    {
        // TODO: Keep counting occurrences of c.
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;


Now, string has an overload of IndexOf that takes a starting position. But where do we start searching? We start searching one position after our last found occurrence.

var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    var i = input.IndexOf(c);
    while (i != -1)
    {
        count++;
        i = input.IndexOf(c, i + 1);
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;


If you prefer, this can be written as a for-loop.

var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    for (var i = input.IndexOf(c); i != -1; i = input.IndexOf(c, i + 1))
    {
        count++;
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;


However, this solution is not terribly efficient. For each character in the string, we're iterating over the string again with our calls to IndexOf, so our time complexity is \$O(n^2)\$.

In fact, we've basically just written this:

var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    foreach (var d in input)
    {
        if (c == d)
        {
            count++;
        }
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;


@BlueTrin's answer shows how you can trade space to reduce the time complexity to \$O(n)\$, so I won't go over that here.

All that said, there is an elegant way to express this using LINQ:

return input.Length == 0
    ? 0
    : input.GroupBy(c => c).Max(group => group.Count());

Code Snippets

Console.WriteLine(GetMaximumOccurence(" "));
var maxCount = 0;
foreach (var c in input)
{
    // TODO: Count occurrences of c in input.
    maxCount = Math.Max(count, maxCount);
}

return maxCount;
var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    var i = input.IndexOf(c);
    while (i != -1)
    {
        // TODO: Keep counting occurrences of c.
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;
var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    var i = input.IndexOf(c);
    while (i != -1)
    {
        count++;
        i = input.IndexOf(c, i + 1);
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;
var maxCount = 0;
foreach (var c in input)
{
    var count = 0;
    for (var i = input.IndexOf(c); i != -1; i = input.IndexOf(c, i + 1))
    {
        count++;
    }

    maxCount = Math.Max(count, maxCount);
}

return maxCount;

Context

StackExchange Code Review Q#58424, answer score: 5

Revisions (0)

No revisions yet.