You should check the input for a valid string:
if (string.IsNullOrWhiteSpace(stringToEncode))
return stringToEncode;
The check for Convert.Count === 0
is a little strange in that Convert
is a private field and only set in one method where we can assume it will be constructed equally every time it is called:
if (Convert == null || Convert.Count == 0) BuildConversionMappings();
But no real harm done, without that check you could just simplify the call like this:
Convert = Convert ?? BuildConversionMappings();
You can set the capacity of the StringBuilder
which is a performance improvement:
StringBuilder sb = new StringBuilder(stringToEncode.Length);
You use continue
a couple of times in your loop:
if (Convert.ContainsKey(c))
{
sb.Append(Convert[c]);
continue;
}
if (char.IsDigit(c))
{
...
If you replace continue
with else if
statements it will IMO give a more readable code:
for (int i = 0; i < stringToEncode.Length; i++)
{
char c = stringToEncode[i];
if (Convert.ContainsKey(c))
{
sb.Append(Convert[c]);
}
else if (char.IsDigit(c))
{
nums.Push(c);
if (i == stringToEncode.Length - 1
|| !char.IsDigit(stringToEncode[i + 1]))
{
while (nums.Count > 0)
{
sb.Append(nums.Pop());
}
}
}
else
{
sb.Append(c);
}
}
That said, I think you main method and main loop are quite good and easy to read and understand, and the Dictionary
idea is just the way to go. But the way you build the dictionary is maybe a little cumbersome.
To the bone you just map between two sets of chars (except for the numbers):
const string keys = "abcdefghijklmnopqrstuvwxyz ";
const string values = "1abc2efg3ijklm4opqrs5uvw yy";
You could look up by finding the index of chars from the input string in keys
and fetch the corresponding value
from values
(lower case). But that would require two look ups per char, and therefore the Dictionary is much better. A Dictionary<char, char>
can be build from the above keys
and values
in the following way:
private static Dictionary<char, char> ConstructMap()
{
const string keys = "abcdefghijklmnopqrstuvwxyz ";
const string values = "1abc2efg3ijklm4opqrs5uvw yy";
IEnumerable<(char c, char cm)> map = keys.Zip(values, (c, cm) => (c, cm));
return map.ToDictionary(ccm => ccm.c, ccm => ccm.cm);
}
because string
implements IEnumerable<char>
Just for the exercise I implemented a quite old school indexed for
-loop like this:
private static readonly Dictionary<char, char> charMap = ConstructMap();
public static string HHEncode(string data)
{
char[] result = new char[data.Length];
for (int i = 0; i < data.Length; i++)
{
if (charMap.TryGetValue(char.ToLower(data[i]), out char value))
{
result[i] = value;
}
else if (char.IsDigit(data[i]))
{
int j = i + 1;
while (j < data.Length && char.IsDigit(data[j])) j++;
j--;
for (int k = 0; k <= (j - i) / 2; k++)
{
result[i + k] = data[j - k];
result[j - k] = data[i + k];
}
i = j;
}
else
{
result[i] = data[i];
}
}
return new string(result);
}
The main difference to yours is how I handle numbers