Leetcode 17 - letter combinations of a phone number - code review conducted by http://codereview.stackexchange.com/a/154888/123986

Leetcode17_letterCombinationOfAPhoneNumber_codereview.cs

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

namespace Leetcode17_CombinationsOfLettersPhoneNumber
{
    /*
     * code review: 
     * http://codereview.stackexchange.com/a/154888/123986
     * 
     * 1. First of all let's move algorithm to a separate class. 
     * 2. Keyboard layout won't change then you should make keyboard a private static field, 
     * you won't even need it as parameter. 
     */
    static class PhoneKeyboard
    {
        private static readonly string[] keyboard = new string[] { "", "", "abc", "def", "ghi", "jkl", "mno", "pqrs", "tuv", "wxyz" };

        /*
         * 1. An empty digit parameter is a corner case for which I wouldn't 
         * perform any speical optimization, let's got through the normal code
         * and return an empty list. 
         */
        public static IEnumerable<string> FindAllWordsForNumber(string digits)
        {
            if (digits == null)
            {
                throw new ArgumentNullException("digits is empty");
            }

            var combinations = new List<string>();
            CombineDigitWithAllSubsequentDigits(digits, 0, new StringBuilder(), combinations);

            return combinations;
        }

        /*
         * code review: 
         * 1. Function name is better to call CombineDigitWithAllSubsequentDigits, 
         * just a recursive function. 
         * 2. Introduce a local boolean to explain its meaning and also consider to use
         * scanIndex for comparison, because it converys more meaning about what you're checking
         * 3. Do not forget to validate input and move that string-to-number conversion 
         * into a seperate function. 
         * 4. Your loop may be simplified little bit using foreach (you do not actually
         * use index) and to trim last character of a stringBuilder you can simple 
         * change its Length property (slightly faster but the point is that IMO it's 
         * more clear the intent.)
         */
        private static void CombineDigitWithAllSubsequentDigits(
            string digitString,
            int scanIndex,
            StringBuilder currentCombination,
            IList<string> letterCombinations
        )
        {
            bool isEndOfString = scanIndex == digitString.Length;  
            if (isEndOfString)
            {
                letterCombinations.Add(currentCombination.ToString());
                return;
            }

            // work on first case, first char in digits array
            int digit = ParseKeyboardDigit(digitString[scanIndex]);           

            foreach (var character in keyboard[digit])
            {                                 
                currentCombination.Append(character);
                CombineDigitWithAllSubsequentDigits(digitString, scanIndex + 1, currentCombination, letterCombinations);

                // Trim last character for the next one in the loop
                --currentCombination.Length;                 
            }
        }

        /*
         * Do not forget to validate input and move that string-to-number conversion into 
         * a separate function
         * 
         */
        public static int ParseKeyboardDigit(char c)
        {
            int digit = c - '0';
            if (digit < 0 || digit >= keyboard.Length)
            {
                throw new ArgumentException(String.Format("Character {0} is not a digit.", c));
            }

            return digit;
        }
    }

    class Leetcode17_CombinationsOfLettersPhoneNumber
    {
        static void Main(string[] args)
        {
            RunTestcase();
            RunTestcase2(); 
        }

        /*
         * Side notes about test cases: 
         * you shouldn't test only for the number of produced combinations but also for
         * their content. It's tedious but it's the only way to be sure about your algorithm. 
         * Also, your tests shouldn't be tied inside the same class where you implement your 
         * algorithm but outside (possibly in a separate assembly for unit testing...)
         */
        public static void RunTestcase()
        {
            // test result: "abc", "def", so 3x3 = 9 cases.
            var letterCombinations = PhoneKeyboard.FindAllWordsForNumber("23");
            Debug.Assert(letterCombinations.Count() == 9);
        }

        public static void RunTestcase2()
        {
            var letterCombinations = PhoneKeyboard.FindAllWordsForNumber("2345678");
            Debug.Assert(letterCombinations.Count() == 9 * 9 * 9 * 4);
        }
    }
}