String: How to find the most common character occurrence.

Why not just output all the ties? 'fo', 2 in your example case.

(...)

The output for said function would be <'o', 2>, it does not present the last letter to appear, only the last in order of UNICODE.

Is that what you are actually looking for?

If it is, here's what i would do...

Everytime you do:

characterCounts[(int)characterToConvert - INDEX_CONVERTER]++;

check if that newly increased value is the new highest... and if it is, update your values to display.

That way the printed character is the last one on the string and you also get rid of that second loop.

I was super confused as to what language you were using. "string" is small so I thought C++, but then I saw you're using "public static string" which smells like java. Then I noticed your function name is capital camel case and your braces aren't Java-like, which could make it C++ again, but most string and array operations are Java again. :S

Anyway, here's my go at solving the problem:

[EDIT] Code removed, turns out it was C# and I thought it was Java.

I was super confused as to what language you were using. "string" is small so I thought C++, but then I saw you're using "public static string" which smells like java. Then I noticed your function name is capital camel case and your braces aren't Java-like, which could make it C++ again, but most string and array operations are Java again. :S

It's C#, given that stitchs has been asking about a C# linked list implementation review in a previous topic.. that and the string.Format() call is a dead giveaway

It's C#, given that stitchs has been asking about a C# linked list implementation review in a previous topic.. that and the string.Format() call is a dead giveaway

Oh. In that case, ignore my post.

The code makes a bad assumption: It assumes the characters in the string are all in the range 32..127.

But chars in C# can be in the range 0..65535. Tabs, newlines, carriage returns are all very common characters will all cause your code to throw an IndexOutOfRangeException.

If your "IsStringValid" method is scanning the string for characters outside that range, then it's fine, but would also waste another loop over the string.

I completely agree Nypyren. This is a self-imposed limitation that I am doing to test with that particular set of Unicode characters. I wanted to refine the method first, and then expand it to include a bigger character set.

Stitchs.

Unless there are predetermined performance constraints, I would always go for a simple solution first:

private static string GetAnyMostFrequentCharacter(string input)
{
    if(!input.Any())
        throw new ArgumentOutOfRangeException("input");
    var group = input.GroupBy(c => c).OrderByDescending(g => g.Count()).First();
    return string.Format("<{0}, {1}>", group.Key, group.Count());
}

Or to get all most frequent characters:

private static string GetAllMostFrequentCharacters(string input)
{
    if (!input.Any())
        throw new ArgumentOutOfRangeException("input");
    var groups = input.GroupBy(c => c).OrderByDescending(g => g.Count());
    var firstLargestGroup = groups.First();
    var allLargestGroups = groups.Where(g => g.Count() == firstLargestGroup.Count());
    var stringBuilder = new StringBuilder();
    foreach (var group in allLargestGroups)
    {
        stringBuilder.AppendLine(string.Format("<{0}, {1}>", group.Key, group.Count()));
    }
    return stringBuilder.ToString();
}

Unless there are predetermined performance constraints, I would always go for a simple solution first:

private static string GetAnyMostFrequentCharacter(string input)
{
    if(!input.Any())
        throw new ArgumentOutOfRangeException("input");
    var group = input.GroupBy(c => c).OrderByDescending(g => g.Count()).First();
    return string.Format("<{0}, {1}>", group.Key, group.Count());
}

Or to get all most frequent characters:

private static string GetAllMostFrequentCharacters(string input)
{
    if (!input.Any())
        throw new ArgumentOutOfRangeException("input");
    var groups = input.GroupBy(c => c).OrderByDescending(g => g.Count());
    var firstLargestGroup = groups.First();
    var allLargestGroups = groups.Where(g => g.Count() == firstLargestGroup.Count());
    var stringBuilder = new StringBuilder();
    foreach (var group in allLargestGroups)
    {
        stringBuilder.AppendLine(string.Format("<{0}, {1}>", group.Key, group.Count()));
    }
    return stringBuilder.ToString();
}

This

For OP.. Linq is your friend, learn to use it and your code complexity will wash away

It looked like an interview type question to me. I think I've even had it asked during an interview. The linq answer is nice for low complexity, but it's not the 'fastest', which I'm guessing the interviewer will want.

I'm not sure why you have two loops. You could keep track of the highest count and index in the first loop, then you wouldn't need to loop through again. (and as someone else pointed out, you're using an array to hold counts, you might want to try a dictionary instead.

And lastly, StringBuilder is more for when you're editing strings, string.concat is better for what you're doing there, which is just concatenating.

http://www.codeproject.com/Articles/14936/StringBuilder-vs-String-Fast-String-Operations-wit

It looked like an interview type question to me. I think I've even had it asked during an interview. The linq answer is nice for low complexity, but it's not the 'fastest', which I'm guessing the interviewer will want.

Depends on the job but during an interview I would be more interested in good grasp of a language and Linq is one of the more complex parts of the language as the lazy evaluation can be used to reduce work if you understand how it really flows. Simple on the skin but you need solid grounding to use it well.

I would say that unless you know absolute speed is what they are after simplicity of code wins every time as easier to change and maintain over long periods :)

Yeah, my first pass at most interview question answers is, "Well, I'm going to do the simple thing first" and then do the most straightforward / easy answer. Then they almost always go, "Okay, but make it faster". But It's still a good idea to show them you know Linq, and/or that simple answers are usually the way to go. (over-optimization is a thing)