Find char which appears maximal number of times

Question

I had 15 minutes to code the following question:

Given a file name return the char which appears maximal number of times.

please do not take into account the unit test. Please look at the final foreach loop, I could have also used a for loop, I have doubts what is the best way to implement finding a max value and get the key from the dictionary.

Also please comment about code style. Thanks

using System.Collections.Generic;
using System.IO;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace JobInterviewTests
{

    [TestClass]
    public class GetMostRepeatedCharQuestion
    {
        [TestMethod]
        public void TestForAasMax()
        {
            char result = GetMaxCharHelper.GetMaxReaptedCharFromFile("MaxA.txt");
            Assert.AreEqual('A', result);
        }
    }

    public class GetMaxCharHelper
    {
        public static char GetMaxReaptedCharFromFile(string fileName)
        {
            Dictionary<char, int> charToCount = new Dictionary<char, int>();
            using (var streamReader = new StreamReader(fileName))
            {
                string line = streamReader.ReadLine();
                while (line != null)
                {
                    char[] charArray = line.ToCharArray();
                    foreach (var currentChar in charArray)
                    {
                        if (charToCount.ContainsKey(currentChar))
                        {
                            charToCount[currentChar]++;
                        }
                        else
                        {
                            charToCount.Add(currentChar, 1);
                        }
                    }
                    //don't forget to get next line when reading from a file
                    line = streamReader.ReadLine();

                }

                int maxCount = 0;
                char maxChar = '\0';
                foreach (var current in charToCount)
                {
                    if (current.Value > maxCount)
                    {
                        maxCount = current.Value;
                        maxChar = current.Key;
                    }
                }
                return maxChar;
            }
        }
    }
}

Answer 1

Assuming modern C#, there are a couple of things you could do to be more concise:

1. Instead of the dance with while((line = reader.NextLine())...) you can just use File.ReadLines and foreach over that.

2. You can declare variables inside the TryGetValue like so: charToCount.TryGetValue(currentChar, out int count) where you can use count afterwards without having to declare it on its own line.

3. While keeping score of your most seen char inside the loop is certainly more efficient (as @pattpass did in their answer), you could also extract the character with the highest count at the end with a still readable oneliner:

   return charToCount.OrderByDescending(cc => cc.Value).First().Key;
   

4. What's completely absent in your method is error handling. What happens if the file does not exist or you aren't allowed to access it? It seems like you handle empty files with returning \0 and 0. It is debatable if that's what one would expect.

All in all, your code could look somewhat likes this:

static char GetMostRepeatedChar(string filename)
{
    // you could also not handle this case here and delegate
    // that responsibility to the caller
    if(!File.Exists(filename))
    {
        return '\0';
    }

    var charCount = new Dictionary<char, int>();
    foreach (var line in File.ReadLines(filename))
    {
        foreach (var c in line)
        {
            charCount.TryGetValue(c, out int count);
            charCount[c] = count + 1;
        }
    }

    if (charCount.Count == 0)
    {
        // this is debatable and depends on the real use case
        // in production code you should never throw a raw 'Exception'
        // but rather something more specific
        throw new Exception("empty file"); 
    }

    return charCount.OrderByDescending(cc => cc.Value).First().Key;
}

Answer 2

According to a modern C# and LINQ you could try to "shock" interviewer with solving in a row:

var res = File.ReadAllText(path)
            .Where(ch => char.IsLetter(ch))
            .GroupBy(x => x)
            .OrderByDescending(x => x.Count())
            .First()
            .Key;