Numbers to words converter

Question

Please review my code:

import java.lang.*;
import java.util.*;
import java.io.*;


class NumberToWords
{
  static public boolean HelperConvertNumberToText(int num, String[] result)
  {
      String [] strones = {
        "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight",
        "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
        "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen",
      };

      String [] strtens = {
          "Ten", "Twenty", "Thirty", "Fourty", "Fifty", "Sixty",
          "Seventy", "Eighty", "Ninety", "Hundred"
      };

      result[0] = "";
      int single, tens, hundreds;

      if(num > 1000)
          return false;

      hundreds = num / 100;
      num = num - hundreds * 100;
      if( num < 20)
      {
          tens = 0; // special case
          single = num;
      }
      else
      {
          tens = num / 10;
          num = num - tens * 10;
          single = num;
      }

      if(hundreds > 0)
      {
         result[0] += strones[hundreds-1];
         result[0] += " Hundred ";
      }
      if(tens > 0)
      {
         result[0] += strtens[tens - 1];
         result[0] += " ";
      }
      if(single > 0)
      {
         result[0] += strones[single - 1];
         result[0] += " ";
      }
      return true;
  }

  static public boolean ConvertNumberToText(int num, String[] result)
  {
      String tempString[] = new String[1];
      tempString[0] = "";
      int thousands;
      int temp;
      result[0] = "";
      if(num < 0 || num > 100000)
      {
          System.out.println(num + " \tNot Supported");
          return false;
      }

      if( num == 0)
      {
         System.out.println(num + " \tZero");
         return false;
      }

      if(num < 1000)
      {  
          HelperConvertNumberToText(num, tempString);
          result[0] += tempString[0];
      }
      else
      {
          thousands = num / 1000;
          temp = num - thousands * 1000;
          HelperConvertNumberToText(thousands, tempString);
          result[0] += tempString[0];
          result[0] += "Thousand ";             
          HelperConvertNumberToText(temp, tempString);
          result[0] += tempString[0];
      }
      return true;
  }

  public static void main(String[] args)
  {
      String [] result = new String[1];
      result[0] = "";
      int  i, num;
      int [] arrNum =
      {
        -1, 0, 5, 10, 15, 19, 20, 21, 25, 33, 49, 50, 72,
        99, 100, 101, 117, 199, 200, 214, 517, 589, 999,
        1000, 1010, 1018, 1200, 9890, 10119, 13535, 57019,
        99999, 100000, 100001
      };

      for (i = 0; i < arrNum.length; i++)
      {
          num = arrNum[i];
          if( ConvertNumberToText(num, result) == true)
             System.out.println(num  + "\t" + result[0]);
      }
  }
}

Answer 1

"Strone"? What's a "strone"? Oh… of course!

In seriousness, naming could be improved. They are obviously arrays of strings; the Hungarian prefix is unnecessary. Also, as constants, they should be declared static final and named with ALL_CAPS.

public class NumberToWords {
    private static final ONES = { "One", "Two", … };
    private static final TENS = { "Ten", "Twenty", … };
    …
}

Helper functions should be private.

It is customary to put the static keyword after the access modifier, as in public static void main(…). It is unconventional to say static public boolean ConvertNumberToText(…). Also, functions should be namedStartingWithLowerCase().

Using a single-element array as a parameter so that you can pass back the result as an out-parameter is very weird. Using a StringBuilder would be a bit more obvious, but still very odd. The right approach should be to return a String, and throw an exception if necessary — perhaps NumberFormatException. Ideally, your code should be able to handle every possible int input, making errors impossible.

Answer 2

Firstly, you have a spelling error. "Fourty" should be "Forty".

Next, You have snippets like this:

num = num - hundreds * 100;

num = num - tens * 10;

temp = num - thousands * 1000;

These can be simplified using the modulo operator. The modulo operator (which is % in java) produces the remainder of a division. For example 1024 % 100 = 24 since 1024 divides into 100 ten times with a remainder of 24.

The snippets are then:

num %= 100;

num %= 10;

temp = num % 1000;

Additionally, you have unnecessary code which can be trimmed down because the results are unused. For instance, you don't use anything from any of the imports... so don't import them.

Lastly, some general code comments:

Why are your convertToString methods... returning booleans? If the arguments are invalid, then raise an Exception, don't return booleans. This would also simplify your code, as it would allow you to simply append the results together. This would also eliminate the need for passing a String[] around.

Also the Helper function should really be called ConvertLessThanThousand indicating what it does.

This is what I came up with:

class NumberToWords
{
  // No need to have these in the function. Unless you really care about hiding them... But reflection can still find these
  private static final String[] ONES = {
     "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight",
     "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
     "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen",
  };

  private static final String[] TENS = {
      "Ten", "Twenty", "Thirty", "Forty", "Fifty", "Sixty",
      "Seventy", "Eighty", "Ninety", "Hundred"
  };

  public String convertLessThanThousand(int num) {

      String result = "";
      int single, tens, hundreds;

      if(num > 1000)
          return result;

      hundreds = num / 100;
      num %= 100;
      if( num < 20) {
          tens = 0;
      }
      else {
          tens = num / 10;
          num %= 10;
      }
      single = num; // Notice I moved this outside since it is duplicated in both cases

      if(hundreds > 0) {
         result += ones[hundreds-1] + " Hundred ";
      }
      if(tens > 0) {
         result += tens[tens - 1] + " ";
      }
      if(single > 0) {
         result += ones[single - 1] + " ";
      }
      return result;
  }

  public static String convertNumberToText(int num)
  {
      String tempString = "";
      int thousands;
      int temp;
      String result = "";
      if(num < 0 || num > 100000) {
          throw new IllegalArgumentException("" + num + " \tNot Supported");
      }

      if( num == 0) {
         return "" + num + " \tZero";
      }

      if(num < 1000) {  
          result += ConvertLessThanThousand(num);
      }
      else {
          thousands = num / 1000;
          temp = num % 1000;
          tempString = ConvertLessThanThousand(thousands);
          result += tempString + "Thousand ";            
          result += ConvertLessThanThousand(temp);
      }
      return result
  }

  public static void main(String[] args)
  {
      int  i, num;
      int[] arrNum =
      {
        -1, 0, 5, 10, 15, 19, 20, 21, 25, 33, 49, 50, 72,
        99, 100, 101, 117, 199, 200, 214, 517, 589, 999,
        1000, 1010, 1018, 1200, 9890, 10119, 13535, 57019,
        99999, 100000, 100001
      };

      for (i = 0; i < arrNum.length; i++)
      {
          num = arrNum[i];
          
          try {
             System.out.println(num + "\t" + ConvertNumberToText(num));
          } catch (IllegalArgumentException e) {
              System.out.println(e.message());
          }
      }
  }
}

Lastly, if you are planning on expanding this to bigger and bigger numbers, I would suggest using a recursive approach, not unlike what I have implemented in Python in my question on this same site.