Counting the amount of repetitions a letter has in a string

Question

I'm working on a Kata on CodeWars in which I have to count the amount of repetitions each letter has in a string. The amount of repetitions should be stored in an int array.

The algorithm I've written seems to almost work, however I get a weird output that I can't explain. I might be missing something in the code.

static void Main(string[] args)
{
    string str = "abcdef";
    string input = str.ToLower();
    int count = 0;

    string[] arrayInput = Regex.Split(input, string.Empty);
    string[] alphabet = Regex.Split("abcdefghijklmnopqrstuvwxyz", string.Empty);
    int[] amounts = new int[input.Length];

    foreach (string letter in alphabet)
    {
        for (int x = 0; x < input.Length; x++)
        {
            if (arrayInput[x] == letter)
            {
               amounts[x]++;
            }
        }
    }

    foreach (int amount in amounts)
    {
        Console.Write(amount + ", ");
    }
    Console.ReadKey();
}

Output:

"2, 1, 1, 1, 1, 1,"

Expected:

"1, 1, 1, 1, 1, 1,"

since each letter only appears once in the string.

Answer 1

When querying, Linq often is good choice:

  using System.Linq;

  ...

  string str = "abcdef";

  // {1, 1, 1, 1, 1, 1} - each letter appears once 
  int[] result = str
    .ToLower()
  //.Where(c => c >= 'a' && c <= 'z') // uncomment, if we want 'a'..'z' range only 
    .GroupBy(c => c)
    .Select(group => group.Count())
    .ToArray();

  Console.Write(string.Join(", ", result));

Answer 2

There are a lot of different approaches, but when it comes to counting of some limited number of items Dictionary is almost always the best choice in terms of performance. The below code is pretty low level if compared to solutions using LINQ, but that's what I like about that: you always control what happens there.

string str = "abcdef";
string input = str.ToLower();

var dict = "abcdefghijklmnopqrstuvwxyz".ToDictionary(k => k, v => 0);

foreach (char c in input)
{
    dict[c]++;
}

var output = new int[dict.Count];   
var index = 0;

foreach (var key in dict.Keys.OrderBy(k => k))
{
    output[index++] = dict[key];
}

In case you want to visualize how the dictionary with counts looks, you can add the following output:

foreach (var key in dict.Keys)
{
    Console.WriteLine($"Key {key} Value {dict[key]}");
}

Answer 3

You have several issues in your code to achieve what you are looking for, like you splitted your str and alphabetby empty string which will give you two extra empty string in your array always! Anyway i think you can do this simply by using Dictionary more efficiently like this way :

string str = "abcdef";
Dictionary<char, int> count_letters = new Dictionary<char, int>();
foreach (var alphabet in str)
{
    if (count_letters.ContainsKey(alphabet))
        count_letters[alphabet] ++;
    else
        count_letters.Add(alphabet, 1);
}

foreach (var result in count_letters)
    Console.WriteLine("{0} - {1}", result.Key, result.Value);

Answer 4

This is your code with some minor corrections, now it works.

  static void Main(string[] args)
        {
            string str = "abbbcdef";
            str = str.ToLower();
            char[] arrayInput = str.ToCharArray();
            char[] alphabet = "abcdefghijklmnopqrstuvwxyz".ToCharArray();
            int[] amounts = new int[str.Length];

            foreach (char letter in arrayInput)
            {
                for (int x = 0; x < alphabet.Length; x++)
                {
                    if (letter.ToString() == alphabet[x].ToString())
                    {
                        amounts[x]++;
                    }
                }
            }
            int numToRemove = 0;
            amounts = amounts.Where(val => val != numToRemove).ToArray();
            foreach (int amount in amounts)
            {
                Console.Write(amount + ", ");
            }
            Console.ReadKey();
        }

Hi

Answer 5

I think you have made a mistake:

int[] amounts = new int[input.Length];

Should be

int[] amounts = new int[26];

And also your loops aren't quite right.

You don't need to split the strings into string arrays. You can just use the string iterator to get each char. Also, if you were doing this on very large strings your solution would be inefficient as for every char you are iterating through the entire alphabet which isn't needed.

you can simplify what you've written considerably:

string input = "abcdef";
int[] counts = new int[26]; 
foreach (var ch in input)
{
    var c = char.ToLower(ch);
    if (c >= 'a' && c <= 'z')
        counts[c - 'a']++;
}

Answer 6

Your regex.split is putting extra slots into your array. Try this:

string[] arrayInput = input.Select(c => c.ToString()).ToArray();
string[] alphabet = "abcdefghijklmnopqrstuvwxyz".Select(c => c.ToString()).ToArray();

Answer 7

When you print the values after the split you will observer the extra space. It seems that is causing issue.

using System;
using System.Text.RegularExpressions;					
public class Program
{
	public static void Main()
	{
		  string str = "abcdef";
    string input = str.ToLower();
    int count = 0;

    char[] arrayInput = input.ToCharArray();
    char[] alphabet = "abcdefghijklmnopqrstuvwxyz".ToCharArray();
    int[] amounts = new int[input.Length];
    
	foreach (char letter in alphabet)
	Console.Write(letter + ", ");
	// observe the first letter here when you use regx.split
	//, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z,	
	
	foreach (char inputWord in arrayInput)
	Console.Write(inputWord + ", ");
	// you get extra space in the start hence there is issue when you use regx.split
	//, a, b, c, d, e, f,
		
    foreach (var letter in alphabet)
    {
        for (int x = 0; x < input.Length; x++)
        {
            if (arrayInput[x] == letter)
            {
               amounts[x] = amounts[x] + 1;
            }
        }
    }

    foreach (int amount in amounts)
    {
        Console.Write(amount + ", ");
    }
   
	}
}

stick to basic: use string.ToCharArray()

Answer 8

Your solution seems overly complicated - the combination of regular expressions, array allocations and nested loops makes it hard to see what is going on.

You can basically implement the solution as a map/reduce variant. First group the characters by their individual characters mapped to the individual characters (the mapping) and then reduce them by taking the count:

var input = "abcdef";
var groups = input.GroupBy(c => c);
var counts = groups.Select(g => g.Count());
Console.WriteLine(string.Join(", ", counts));