LINQ and string.Split do it yourself practice

Question

Problem Statement:

Implement a sentence scanning functionality to parse the sentence and return a string to concatenate each word with its number of occurrences, sorted by the number of occurrences in descending order.

For example, with the sentence "code review connects the world! share the code" , the function returns the string: {code: 2, the: 2, review: 1, connects: 1, world: 1, share: 1}. The delimiters are customized as function input argument, for the above test case, the delimiters are " ,!".

Introduction of algorithm

I spent over an hour to write the algorithm first with time complexity \$O(nm)\$, where \$n\$ is the sentence's length and \$m\$ is delimiters length. I practiced using C#, tried to write a string.Split(string) by myself, and also learn to write LINQ for correct syntax by looking up the stackoverflow question to make the code more succinct.

I am still learning to write readable and clean code. The code passes two test case function calls in the main function. Please help me to improve.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;

class ScanArticle
{
    static void Main(string[] args)
    {       
        RunTestcaseOnSplit();
        RunTestcase();
    }

    public static void RunTestcase()
    {
        var result = WordCountPractice(" code review connects the world! share the code", " ,?!");
        Debug.Assert(result.CompareTo("{code: 2, the: 2, review: 1, connects: 1, world: 1, share: 1}") == 0);
    }

    /// <summary>
    /// test do it yourself string.Split(string)
    /// Test case: " a,b,c", splitted words: new string[]{"a","b","c"}
    /// Test case: " a,b,c,", splitted words: new string[]{"a","b","c"} 
    /// </summary>
    public static void RunTestcaseOnSplit()
    {
        var words = stringSplitDoItYourSelf("  a,b,c", " ,");
        Debug.Assert(words[0].CompareTo("a") == 0);

        var words2 = stringSplitDoItYourSelf("  a,b,c,", " ,");
        Debug.Assert(words[2].CompareTo("c") == 0);
    }


    /// <summary>    
    /// words: " code review connects the world! share the code"
    /// the function returns the string: {code: 2, the: 2, review: 1, connects: 1, world: 1, share: 1}.
    /// Ignore punctuation and white-space. 
    /// 
    /// minimum run time O(N) + O(NM), N is words' length, and M is delimiters' length 
    /// </summary>
    /// <param name="words"></param>
    /// <param name="delimiters"></param>
    /// <returns></returns>
    public static string WordCountPractice(string words, string delimiters)
    {
        IList<string> sortedWords = new List<string>();

        if (words == null || words.Length == 0)
        {
            return string.Empty;
        }

        var split = stringSplitDoItYourSelf(words, delimiters);

        var original = new Dictionary<string, int>();

        foreach (var s in split)
        {
            if (original.ContainsKey(s))
            {
                original[s]++;
            }
            else
            {
                original.Add(s, 1);
            }
        }

        // time O(NlogN)      
        var sorted = from entry in original orderby entry.Value descending select entry;

        concatenateContent(sortedWords, sorted);         

        return string.Join("", sortedWords.ToArray());
    }

    /// <summary>
    /// wordCount: code, 2; the, 2
    /// the output: {code: 2, the: 2}
    /// </summary>
    /// <param name="words"></param>
    /// <param name="wordCount"></param>
    private static void concatenateContent(IList<string> words, IEnumerable<KeyValuePair<string, int>> wordCount)
    {
        const string openParenthese = "{";
        const string closeParenthese = "}";
        const string colon = ": ";
        const string comma = ", ";

        words.Add(openParenthese);

        int length = wordCount.Count();
        int index = 0;

        foreach (var item in wordCount)
        {
            StringBuilder current = new StringBuilder();

            current.Append(item.Key + colon + item.Value);

            if (index < length - 1)
            {
                current.Append(comma);
            }

            words.Add(current.ToString());

            index++;
        }

        words.Add(closeParenthese);
    }

    /// <summary>
    /// delimter =" ;-/"
    /// delimiter string length is m
    /// n is string s's length
    /// algorithm time complexity is O(nm) 
    /// 
    /// Do it yourself version of string.Split(char[])
    /// </summary>
    /// <param name="sentence"></param>
    /// <param name="delimiters"></param>
    /// <returns></returns>
    private static string[] stringSplitDoItYourSelf(string sentence, string delimiters)
    {
        IList<string> words = new List<string>();

        if (sentence == null || sentence.Length == 0)
        {
            return new string[0];
        }

        int length = sentence.Length;
        int start = 0;
        int end = 0;

        for (int i = 0; i < length; i++)
        {
            var current = sentence[i];

            var startChar = sentence[start];

            // continue to skip delimiter char if need
            if (delimiters.IndexOf(startChar) >= 0)
            {
                start = i;
                if (end < start)
                {
                    end = start;
                }
            }

            if (delimiters.IndexOf(current) >= 0)
            {
                end = i;

                // output the word, s[end] is delimiter 
                if (end - start > 0)
                {
                    words.Add(sentence.Substring(start, end - start));               
                    start = end + 1;
                }
            }
        }

        // edge case
        if (start < length && delimiters.IndexOf(sentence[start]) < 0)
        {
            words.Add(sentence.Substring(start, length - start));
        }

        return words.ToArray();
    }
}

Answer 1

There is not much LINQ in your code and this is a really long solution for such a simple task but let's try to change it.

You want to split a sentence like this one:

var sentence = " code review connects the world! share the code";

so why not use regex for this and split on every occurance of ,?!.. You then trim each word and filter the empty results out. Then you group each word igrnoring its case, sort the groups by count in a descending order and create strings.

var words = 
    Regex.Split(sentence, "[ ,?!.]")
    .Select(x => x.Trim())
    .Where(x => !string.IsNullOrEmpty(x))
    .GroupBy(x => x, StringComparer.OrdinalIgnoreCase)
    .OrderByDescending(g => g.Count());

var summary = $"{{{string.Join(", ", words.Select(g => $"{g.Key}: {g.Count()}"))}}}";

result

{code: 2, the: 2, review: 1, connects: 1, world: 1, share: 1}

---

Review

private static void concatenateContent(IList<string> words, IEnumerable<KeyValuePair<string, int>> wordCount)
{
    const string openParenthese = "{";
    const string closeParenthese = "}";
    const string colon = ": ";
    const string comma = ", ";

    words.Add(openParenthese);

    int length = wordCount.Count();
    int index = 0;

    foreach (var item in wordCount)
    {
        StringBuilder current = new StringBuilder();

        current.Append(item.Key + colon + item.Value);

        if (index < length - 1)
        {
            current.Append(comma);
        }

        words.Add(current.ToString());

        index++;
    }

    words.Add(closeParenthese);
}

You should not return a result via a parameter that is not a ref or out one because I'd be really shocked if you modified my list without giving me any hint that it will happen.

The IList<string> words should be a return value not a parameter.

Using the StringBuilder current = new StringBuilder(); inside a loop does not make much sense and you use the + concatenation anyway item.Key + colon + item.Value. You might as well concatenate the string in the normal way because the comptiler will optimize it. The StringBuilder will become useful when you define it outside the loop and then build the string.

You don't need the index++; because you can just check the words.Count property.

Ideally this method could look like this:

private static string concatenateContent(IEnumerable<KeyValuePair<string, int>> wordCount)
{
    const string openParenthese = "{";
    const string closeParenthese = "}";
    const string colon = ": ";
    const string comma = ", ";

    var result = new StringBuilder();
    result.Append(openParenthese);

    foreach (var x in wordCount.Select((item, index) => new { item, index }))
    {
        result
            .Append(x.index > 0 ? colon : string.Empty)
            .Append(x.item.Key)
            .Append(colon)
            .Append(x.item.Value);
    }

    result.Append(closeParenthese);

    return result.ToString();
}

Answer 2

public static void RunTestcase()
{
    var result = WordCountPractice(" code review connects the world! share the code", " ,?!");
    Debug.Assert(result.CompareTo("{code: 2, the: 2, review: 1, connects: 1, world: 1, share: 1}") == 0);
}

There are tests!!! +1000

I'm very excited to see you using tests, but do yourself a favor and learn to use a unit testing framework (MSTest is built into Visual Studio.) Some would say keeping the tests in the same class as the code gives the class 2 responsibilities.