Number to Word Application in C#

Question

I've created a Windows Form Number to Word converter application in C# which converts any number from 0 to 1 million. I would love a review based on efficiency and best practices.

MainInterface.cs

using System;
using System.Windows.Forms;
namespace NumberToWords
{
 public partial class MainInterface : Form
 {
 private NumberToWordsConverter numberToWordsConverter;
 public MainInterface()
 {
 InitializeComponent();
 }
 private void MainInterface_Load(object sender, EventArgs e)
 {
 this.numberToWordsConverter = new NumberToWordsConverter();
 }
 private void button1_Click(object sender, EventArgs e)
 {
 string input = textBox1.Text;
 int numberToConvert;
 if (int.TryParse(input, out numberToConvert))
 {
 if (numberToConvert < 0 || numberToConvert > 1000000)
 {
 label3.Text = "Invalid range, please enter a number between 0 and 1 million inclusively";
 } else
 {
 string stringData = this.numberToWordsConverter.convertToString(numberToConvert);
 label3.Text = stringData;
 }
 }
 else
 {
 label3.Text = "Please enter a valid number";
 }
 }
 }
}

NumberToWordsConverter.cs

using System;
namespace NumberToWords
{
 public class NumberToWordsConverter
 {
 private string[] numbers = { "Zero", "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen", "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen" };
 private string[] tens = { "", "Ten", "Twenty", "Thirty", "Forty", "Fifty", "Sixty", "Seventy", "Eighty", "Ninety" };
 public NumberToWordsConverter()
 {
 }
 public string convertToString(int number)
 {
 char[] digits = number.ToString().ToCharArray();
 string words = null;
 if (number >= 0 && number <= 19)
 {
 words = words + this.numbers[number];
 }
 else if (number >= 20 && number <= 99)
 {
 int firstDigit = (int)Char.GetNumericValue(digits[0]);
 int secondPart = number % 10;
 words = words + this.tens[firstDigit];
 if (secondPart > 0)
 {
 words = words + " " + convertToString(secondPart);
 }
 }
 else if (number >= 100 && number <= 999)
 {
 int firstDigit = (int)Char.GetNumericValue(digits[0]);
 int secondPart = number % 100;
 words = words + this.numbers[firstDigit] + " hundred";
 if (secondPart > 0)
 {
 words = words + " and " + convertToString(secondPart);
 }
 }
 else if (number >= 1000 && number <= 19999)
 {
 int firstPart = (int)Char.GetNumericValue(digits[0]);
 if (number >= 10000)
 {
 string twoDigits = digits[0].ToString() + digits[1].ToString();
 firstPart = Convert.ToInt16(twoDigits);
 }
 int secondPart = number % 1000;
 words = words + this.numbers[firstPart] + " thousand";
 if (secondPart > 0 && secondPart <= 99)
 {
 words = words + " and " + convertToString(secondPart);
 } else if (secondPart >= 100)
 {
 words = words + " " + convertToString(secondPart);
 }
 }
 else if (number >= 20000 && number <= 999999)
 {
 string firstStringPart = Char.GetNumericValue(digits[0]).ToString() + Char.GetNumericValue(digits[1]).ToString();
 if (number >= 100000)
 {
 firstStringPart = firstStringPart + Char.GetNumericValue(digits[2]).ToString();
 }
 int firstPart = Convert.ToInt16(firstStringPart);
 int secondPart = number - (firstPart * 1000);
 words = words + convertToString(firstPart) + " thousand";
 if (secondPart > 0 && secondPart <= 99)
 {
 words = words + " and " + convertToString(secondPart);
 } else if (secondPart >= 100)
 {
 words = words + " " + convertToString(secondPart);
 }
 }
 else if (number == 1000000)
 {
 words = words + "One million";
 }
 return words;
 }
 }
}

Answer 1

NumberToWordsConverter.convertToString(int number)

• The validation of he passed number shouldn't be done by the caller of the method but by the class itself. You could signal the range of allowed numbers as Maximum and Minimum either as properties or as public readonly fields.

  If we change the start of the method from

  public string convertToString(int number)
  {
   char[] digits = number.ToString().ToCharArray();
   string words = null;
   if (number >= 0 && number <= 19) 
  

  to this

  public string ConvertToString(int number)
  {
   string words = null;
   if (number < 0)
   {
   number *= -1;
   words = "minus ";
   }
   char[] digits = number.ToString().ToCharArray();
   if (number <= 19) 
  

  we can extend the minimum range from 0 to -1000000. The validation of the passed number based on this is for you to do.

• Based on the naming guidelines methods should be named using PascalCase casing. So convertToString() should be named ConvertToString()

• Using a static method instead of an instance method like mentioned by Xiaoy312 is a have to in this case.

• Adding up strings by using + will result each time in creating a new string object because strings are immutable. This can be improved by using a StringBuilder. I would change this method to have an additional StringBuilder argument which is then called by an overladed ConvertToNumber() method like so

  private static void EnsureNumberIsInRange(int number)
  {
   // to be filled by you
  }
  public static string ConvertToString(int number)
  {
   EnsureNumberIsInRange(number);
   if (number == 0)
   {
   return numbers[number];
   }
   StringBuilder builder = new StringBuilder(1024);
   if (number < 0)
   {
   number *= -1;
   builder.Append("minus ");
   }
   ConvertToString(number, builder);
   return builder.ToString().TrimEnd();
  } 
  

  By handling the case if the passed in number equals 0 we can use this case in the recursivly called overloaded method as guard clause.

  If we change

  char[] digits = number.ToString().ToCharArray(); 
  

  to

  string digits = number.ToString(); 
  

  we remove the call to ToCharArray() for saving some memory.

  By extracting the edge cases for number >= 1000 && number <= 19999 and number >= 20000 && number <= 999999 to separate if statements we can clean this up some more which results in this

  private static void ConvertToString(int number, StringBuilder builder)
  {
   if (number == 0) { return; }
   if (number <= 19)
   {
   builder.Append(numbers[number]);
   return;
   }
   string digits = number.ToString();
   int firstDigit = (int)Char.GetNumericValue(digits[0]);
   if (number <= 99)
   {
   builder.Append(tens[firstDigit])
   .Append(" ");
   number = number % 10;
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999)
   {
   builder.Append(numbers[firstDigit])
   .Append(" hundred ");
   number = number % 100;
   if (number > 0)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   if (number <= 9999)
   {
   builder.Append(numbers[firstDigit])
   .Append(" thousand ");
   number = number % 1000;
   if (number > 0 && number <= 99)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   if (number <= 19999)
   {
   int firstPart = firstDigit * 10 + (int)Char.GetNumericValue(digits[1]);
   builder.Append(numbers[firstPart])
   .Append(" thousand ");
   number = number % 1000;
   if (number > 0 && number <= 99)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   if (number <= 99999)
   {
   int firstPart = firstDigit * 10 + (int)Char.GetNumericValue(digits[1]);
   ConvertToString(firstPart, builder);
   builder.Append(" thousand ");
   number = number % 1000;
   if (number > 0 && number <= 99)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999999)
   {
   int firstPart = firstDigit * 100 + (int)Char.GetNumericValue(digits[1]) * 10 + (int)Char.GetNumericValue(digits[2]);
   ConvertToString(firstPart, builder);
   builder.Append(" thousand ");
   number = number % (firstPart * 1000);
   if (number > 0 && number <= 99)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   builder.Append("One million");
  }
  

  But we still have some small code duplication here regarding

  if (number > 0 && number <= 99)
  {
   builder.Append("and ");
  }
  

  by extracting this to a method like so

  private static void AppendAndIfNeeded(int number, StringBuilder builder)
  {
   if (number > 0 && number <= 99)
   {
   builder.Append("and ");
   }
  }
  

  leading to this

  private static void ConvertToString(int number, StringBuilder builder)
  {
   if (number == 0) { return; }
   if (number <= 19)
   {
   builder.Append(numbers[number]);
   return;
   }
   string digits = number.ToString();
   int firstDigit = (int)Char.GetNumericValue(digits[0]);
   if (number <= 99)
   {
   builder.Append(tens[firstDigit])
   .Append(" ");
   number = number % 10;
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999)
   {
   builder.Append(numbers[firstDigit])
   .Append(" hundred ");
   number = number % 100;
   if (number > 0)
   {
   builder.Append("and ");
   }
   ConvertToString(number, builder);
   return;
   }
   if (number <= 9999)
   {
   builder.Append(numbers[firstDigit])
   .Append(" thousand ");
   number = number % 1000;
   AppendAndIfNeeded(number, builder);
   ConvertToString(number, builder);
   return;
   }
   if (number <= 19999)
   {
   int firstPart = firstDigit * 10 + (int)Char.GetNumericValue(digits[1]);
   builder.Append(numbers[firstPart])
   .Append(" thousand ");
   number = number % 1000;
   AppendAndIfNeeded(number, builder);
   ConvertToString(number, builder);
   return;
   }
   if (number <= 99999)
   {
   int firstPart = firstDigit * 10 + (int)Char.GetNumericValue(digits[1]);
   ConvertToString(firstPart, builder);
   builder.Append(" thousand ");
   number = number % 1000;
   AppendAndIfNeeded(number, builder);
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999999)
   {
   int firstPart = firstDigit * 100 + (int)Char.GetNumericValue(digits[1]) * 10 + (int)Char.GetNumericValue(digits[2]);
   ConvertToString(firstPart, builder);
   builder.Append(" thousand ");
   number = number % (firstPart * 1000);
   AppendAndIfNeeded(number, builder);
   ConvertToString(number, builder);
   return;
   }
   builder.Append("One million");
  }
  

  By using integer division instead of the calls of the numerous (int)Char.GetNumericValue() methods we can make the code more clear and can get rid of the string digits and as bonus we won't need to handle the edge cases anymore like so

  private static void ConvertToString(int number, StringBuilder builder)
  {
   if (number == 0) { return; }
   if (number <= 19)
   {
   builder.Append(numbers[number]);
   return;
   }
   if (number <= 99)
   {
   builder.Append(tens[number/10])
   .Append(" ");
   number = number % 10;
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999)
   {
   ConvertToString(number / 100, builder);
   builder.Append(" hundred ");
   number = number % 100;
   AppendAndIfNeeded(number, builder);
   ConvertToString(number, builder);
   return;
   }
   if (number <= 999999)
   {
   ConvertToString(number / 1000, builder);
   builder.Append(" thousand ");
   number = number % 1000;
   AppendAndIfNeeded(number / 1000, builder);
   ConvertToString(number, builder);
   return;
   }
   builder.Append("One million");
  }
  

• codez: Thanks for your effort and feel free to do so tomorrow as well. I will tick an answer after 72 hours since the time I posted this question.
• Heslacher: For me the "tomorrow" is now ;-)
• codez: Hahahaha, okay okay. ;P

Answer 2

Design

NumberToWordsConverter is a helper/utility class here, so it is best to make it a static class. That way you don't have to instantiate it before using it. It is not like you have various converters that will be used depending on the context like localized converters: English, French, Chinese...

public static class NumberToWordsConverter
{
 // I also marked them readonly since they are not meant to be changed.
 private static readonly string[] Numbers = { ... };
 private static readonly string[] Tens = { ... };
 public static string convertToString(int number) { ... }
}

Naming Convention

DO use PascalCasing for all public member, type, and namespace names consisting of multiple words.

src : https://msdn.microsoft.com/en-us/library/vstudio/ms229043(v=vs.110).aspx

public static string ConvertToString(int number)

Defensively Coding

While you validated the user input on the UI layer, MainInterface , it is also important to validate the argument passed on your business/service layer, NumberToWordsConverter.

public static string ConvertToString(int number)
{
 const int MinimumRange = 0, MaximumRange = 1000000;
 if (MinimumRange > number || number > MaximumRange)
 throw new ArgumentOutOfRangeException("number", number,
 string.Format("Only number between {0} and {1}", MinimumRange, MaximumRange));
 // ...
}

Negative Number

public static string ConvertToString(int number);

int is a pretty wide range, not only does it holds positive numbers up to 2,147,483,647, but also negative numbers down to -2,147,483,648. If we pass any negative number, the method simply returns null.

There is 3 ways of fixing it :

1. Throw an exception on negative numbers. This is covered in last section.
2. Change the parameter to uint.

3. Do a custom routine for negative numbers :

   public static string ConvertToString(int number)
   {
    if (number < 0)
    return "Minus " + ConvertToString(-number);
    // ...
   }
   

Output

90 Ninety 
91 Ninety One 
92 Ninety Two 
93 Ninety Three 
94 Ninety Four 
95 Ninety Five 
96 Ninety Six 
97 Ninety Seven 
98 Ninety Eight 
99 Ninety Nine 
100 One hundred 
101 One hundred and One 
102 One hundred and Two 
103 One hundred and Three 
104 One hundred and Four 
105 One hundred and Five 
106 One hundred and Six 
107 One hundred and Seven 
108 One hundred and Eight 
109 One hundred and Nine 
110 One hundred and Ten

I don't know if you have noticed the inconsistency. All units and tens are in title case, while the names of large number are in lower case. Consider making them all lower case.

Answer 3

I'd like to show another way to spell numbers. It works like we think by dividing the whole number in sets of three digits and translating each into words. Finally it adds a thousands multiplier to each triplet if necessary. This is a generic method and can very easily be extended by just adding another thousands multiplier. It lacks range checking etc. but it's purpose is to just show another possible sollution.

Putting it in a sigle class might not be the best idea but here it is:

public static class NumberSpeller
{
 public static string SpellNumber(long number)
 {
 // parse digits and reverse their order
 var digits = number.ToString().Select(digit => int.Parse(digit.ToString())).Reverse().ToArray();
 var numberNames = new List<string>();
 // calc the number of triplets
 var tripletCount = (int)Math.Ceiling(digits.Length / 3.0);
 for (var tripletNumber = 0; tripletNumber < tripletCount; tripletNumber++)
 {
 var triplet = Triplet.ToText(digits, tripletNumber);
 if (string.IsNullOrEmpty(triplet))
 {
 continue;
 }
 // add thousands multiplier if there is more then one triplet and we already have the first one
 if (tripletCount > 0 && tripletNumber > 0)
 {
 numberNames.Add(NumberNames.Triplets[tripletNumber]);
 }
 numberNames.Add(triplet);
 }
 // join all triplets
 var result = string.Join(" ", numberNames.AsEnumerable().Reverse());
 return result;
 }
 private class Triplet
 {
 private readonly int[] _digits;
 private readonly int _tripletNumber;
 private Triplet(int[] digits, int tripletNumber)
 {
 _digits = digits;
 _tripletNumber = tripletNumber;
 }
 private bool HasOnes { get { return Length >= 1; } }
 private bool HasTens { get { return Length >= 2; } }
 private bool HasHundreds { get { return Length == 3; } }
 private int Ones
 {
 get { return _digits[0]; }
 }
 private int Tens
 {
 get { return _digits[1]; }
 }
 private int Hundreds
 {
 get { return _digits[2]; }
 }
 public int Length
 {
 get { return _digits.Length; }
 }
 public override string ToString()
 {
 var result = new List<string>();
 if (HasHundreds && Hundreds > 0)
 {
 // appent multiplier
 result.Add(NumberNames.Ones[Hundreds]);
 // append hundred
 result.Add(NumberNames.Triplets[0]);
 }
 if (HasTens && Tens > 0)
 {
 // tens are beetween 11 and 19
 if (Tens > 0 && Tens < 2)
 {
 result.Add(NumberNames.Teens[Ones]);
 }
 // tens are greater then 19
 else if (Tens >= 2)
 {
 result.Add(NumberNames.Tens[Tens - 2]);
 }
 }
 if (HasOnes)
 {
 // append zero only if there is one triplet and a single digit
 if (Ones == 0 && Length == 1 && _tripletNumber == 0)
 {
 result.Add(NumberNames.Ones[Ones]);
 }
 else if (Length == 1 || (Length == 3 && Tens == 0 && Ones != 0))
 {
 result.Add(NumberNames.Ones[Ones]);
 }
 else if (HasTens && Tens >= 2 && Ones > 0)
 {
 result.Add(NumberNames.Ones[Ones]);
 }
 }
 return string.Join(" ", result);
 }
 // gets digits for a triplet
 private static int[] GetDigits(IReadOnlyCollection<int> digits, int tripletNumber)
 {
 var from = tripletNumber * 3;
 var to = tripletNumber * 3 + 2;
 var lastIndex = digits.Count - 1;
 if (from > lastIndex)
 {
 return null;
 }
 if (to > lastIndex)
 {
 to = lastIndex;
 }
 var tripletDigits = digits.Where((digit, i) => i >= from && i <= to).ToArray();
 return tripletDigits;
 }
 public static string ToText(int[] digits, int tripletNumber)
 {
 var tripletDigits = GetDigits(digits, tripletNumber);
 var triplet = new Triplet(tripletDigits, tripletNumber);
 return triplet.ToString();
 }
 }
 public static class NumberNames
 {
 public static readonly string[] Ones =
 {
 "Zero",
 "One",
 "Two",
 "Three",
 "Four",
 "Five",
 "Six",
 "Seven",
 "Eight",
 "Nine",
 };
 public static readonly string[] Teens =
 {
 "Ten",
 "Eleven",
 "Twelve",
 "Thirteen",
 "Fourteen",
 "Fifteen",
 "Sixteen",
 "Seventeen",
 "Eighteen",
 "Nineteen"
 };
 public static readonly string[] Tens =
 {
 "Twenty",
 "Thirty",
 "Forty",
 "Fifty",
 "Sixty",
 "Seventy",
 "Eighty",
 "Ninety"
 };
 // this allows to easily extend the spellable number
 public static readonly string[] Triplets =
 {
 "Hundred",
 "Thousand",
 "Million",
 "Billion",
 "Trillion",
 };
 }
}

Usage:

private static void Main(string[] args)
{
 var numbers = new long[]
 {
 0,
 1,
 10,
 99,
 100,
 101,
 119,
 229,
 1001,
 1397,
 1000003,
 34345231,
 3100002201,
 };
 var texts = numbers.Select(n => n.ToString("N0") + " = " + NumberSpeller.SpellNumber(n));
 foreach (var text in texts)
 {
 Debug.WriteLine(text);
 Console.WriteLine(text);
 }
 //Console.ReadKey();
}

Result:

0 = Zero
1 = One
10 = Ten
99 = Ninety Nine
100 = One Hundred
101 = One Hundred One
119 = One Hundred Nineteen
229 = Two Hundred Twenty Nine
1.001 = One Thousand One
1.397 = One Thousand Three Hundred Ninety Seven
1.000.003 = One Million Three
34.345.231 = Thirty Four Million Three Hundred Forty Five Thousand Two Hundred Thirty One
3.100.002.201 = Three Billion One Hundred Million Two Thousand Two Hundred One

• codez: Well, my example includes 'and' and stuff.