There is only one thing I really don't like:
You have the same array instantiated in two different places (the phone/ph
array). Here it's fairly simple and easy to maintain, but IRL you should never do that.
You have two options to resolve this:
1) Create the phone
array as a class field or property.
2) In the newest versions of C# you can place the recursive GetCombos(...)
as an internal/local function inside LetterCombinations(...)
and then only have string[] phone = {...}
defined in the outer:
IList<string> LetterCombinations(...)
{
string[] phone = { ... };
void GetCombos(...)
{
...
char ch = phone[inputDigits[0] - '0'][charIndex];
...
}
int len = phone[digits[0] - '0'].Length;
...
}
As shown the inner function can access the variables defines in the containing method.
Other things:
public void GetCombos(string inputDigits, string curVariation, int charIndex, ref List<string> resultList, int length) { ... }
You don't need the ref
keyword because a list is a reference type.
I don't think newInput
will ever be empty, so you can skip this check:
if (newInput == "") return;
I would make this test
if (digits == null || digits.Length == 0) return result;
as the first thing to do in the method:
if (string.IsNullOrWhiteSpace(digits)) return new List<string>();
(or maybe throw an exception?)
In GetCombos(...)
you could use the yield return
concept instead of the resultList argument as shown below:
public IList<string> LetterCombinations(string digits)
{
if (digits == null || digits.Length == 0) return new List<string>();
string[] phone = new string[] { "0", "1", "abc", "def", "ghi", "jkl", "mno", "pqrs", "tuv", "wxyz" };
List<string> result = new List<string>();
IEnumerable<string> GetCombos(string inputDigits, string curVariation, int charIndex, int length)
{
if (curVariation.Length != length)
{
char ch = phone[inputDigits[0] - '0'][charIndex];
curVariation += ch;
}
if (curVariation.Length == length)
{
yield return curVariation;
}
else
{
string newInput = inputDigits.Substring(1, inputDigits.Length - 1);
int numChars = phone[newInput[0] - '0'].Length;
for (int i = 0; i < numChars; i++)
{
foreach (string variation in GetCombos(newInput, curVariation, i, length))
{
yield return variation;
}
}
}
}
int len = phone[digits[0] - '0'].Length;
for (int i = 0; i < len; i++)
{
result.AddRange(GetCombos(digits, "", i, digits.Length));
}
return result;
}
The LeetCode challenge states that the return value of LetterCombinations(...)
should be IList<string>
, but it would be more elegant to let it return IEnumerable<string>
as well and then use yield return
insted of result.AddRange(GetCombos(...))
(you'll have to make it in a foreach ()
loop though).
Just for the exercise I made my own version:
IEnumerable<string> LetterCombinations(string digits)
{
string[] phone = new string[] { "0", "1", "abc", "def", "ghi", "jkl", "mno", "pqrs", "tuv", "wxyz" };
int length = digits.Length;
IEnumerable<string> Runner(int buttonIndex, int digitIndex)
{
if (buttonIndex < 2 || buttonIndex > 9)
throw new ArgumentOutOfRangeException(nameof(digits));
if (digitIndex == length)
{
foreach (char ch in phone[buttonIndex])
yield return ch.ToString();
}
else
{
int newButtonIndex = digits[digitIndex] - '0';
digitIndex++;
foreach (char ch in phone[buttonIndex])
{
foreach (string tail in Runner(newButtonIndex, digitIndex))
{
yield return $"{ch}{tail}";
}
}
}
}
return Runner(digits[0] - '0', 1);
}
Not to claim it to be better than yours - in fact it seems to be slightly slower - but just to show another approach.
In fact it can be done in one line using LINQ:
IEnumerable<string> LetterCombinations(string digits)
{
string[] phone = new string[] { "0", "1", "abc", "def", "ghi", "jkl", "mno", "pqrs", "tuv", "wxyz" };
return digits.Skip(1).Select(d => d - '0').Aggregate(phone[digits[0] - '0'].Select(c => c.ToString()), (acc, i) => phone[i].SelectMany(c => acc.Select(a => $"{a}{c}")));
}