Converting an integer number into its word representation

Question

I've written this code to convert an integer number into its word representation. I've tested it for many cases and it seems to be working as expected.

For example:

• 1000 displayed as "One Thousand"
• 99999 displayed as "Ninety nine thousand nine hundred ninety nine"
• 999999999 displayed as "Nine hundred ninety nine million nine hundred ninety nine thousand nine hundred ninety nine"

However, I think it could be improved especially because I'm doing some checks repeatedly. Can someone suggest some refactoring for this?

import java.util.HashMap;
public class NumberToWord {
 static HashMap<Integer,String> numberMap = new HashMap<Integer,String>();
 static{
 numberMap.put(0,"Zero");
 numberMap.put(1,"One");
 numberMap.put(2,"Two");
 numberMap.put(3,"Three");
 numberMap.put(4,"Four");
 numberMap.put(5,"Five");
 numberMap.put(6,"Six");
 numberMap.put(7,"Seven");
 numberMap.put(8,"Eight");
 numberMap.put(9,"Nine");
 numberMap.put(10,"Ten");
 numberMap.put(11,"Eleven");
 numberMap.put(12,"Twelve");
 numberMap.put(13,"Thirteen");
 numberMap.put(14,"Fourteen");
 numberMap.put(15,"Fifteen");
 numberMap.put(16,"Sixteen");
 numberMap.put(17,"Seventeen");
 numberMap.put(18,"Eighteen");
 numberMap.put(19,"Nineteen");
 numberMap.put(20,"Twenty");
 numberMap.put(30,"Thirty");
 numberMap.put(40,"Forty");
 numberMap.put(50,"Fifty");
 numberMap.put(60,"Sixty");
 numberMap.put(70,"Seventy");
 numberMap.put(80,"Eighty");
 numberMap.put(90,"Ninety");
 numberMap.put(100,"Hundred");
 numberMap.put(1000,"Thousand");
 }
 public static String numberToWord(int number)
 {
 String wordForm = "";
 int quotient =0;
 int remainder = 0;
 int divisor = 0;
 if(number<1000000000 && number>=1000000)
 {
 divisor = 1000000;
 quotient = number/divisor;
 remainder = number % divisor;
 if(quotient!=0)
 wordForm += numberToWord(quotient) + " " + "Million";
 if(remainder!=0)
 wordForm+= " " + numberToWord(remainder);
 }
 else if(number<1000000 && number>=10000)
 {
 divisor = 1000;
 quotient = number/divisor;
 remainder = number % divisor;
 if(quotient!=0)
 wordForm += numberToWord(quotient) + " " + "Thousand";
 if(remainder!=0)
 wordForm+= " " + numberToWord(remainder);
 }
 else if(number<10000 && number>=1000)
 {
 divisor = 1000;
 quotient = number/divisor;
 remainder = number % divisor;
 if(quotient!=0)
 wordForm += numberMap.get(quotient) + "Thousand";
 if(remainder!=0)
 wordForm+= numberToWord(remainder);
 }else if(number<1000 && number>=100)
 {
 divisor = 100;
 quotient = number/divisor;
 remainder = number % divisor;
 if(quotient!=0)
 wordForm += numberMap.get(quotient) + " " + "Hundred";
 if(remainder!=0)
 wordForm +=numberToWord(remainder);
 }else if(number<100 && number>=10)
 {
 divisor = 10;
 quotient = number/divisor;
 remainder = number % divisor;
 if(quotient!=0)
 wordForm+= numberMap.get(quotient*divisor);
 if(remainder!=0)
 wordForm+= numberMap.get(remainder);
 }else if(number<10 && number>=0)
 {
 wordForm += numberMap.get(number);
 }
 return wordForm;
 }
 public static void main(String[] args)
 {
 System.out.println(numberToWord(100000));
 System.out.println(numberToWord(999999));
 System.out.println(numberToWord(678900));
 System.out.println(numberToWord(0));
 System.out.println(numberToWord(100567));
 System.out.println(numberToWord(4589));
 System.out.println(numberToWord(3333));
 System.out.println(numberToWord(67500));
 System.out.println(numberToWord(72));
 System.out.println(numberToWord(172346));
 System.out.println(numberToWord(890000));
 System.out.println(numberToWord(600700));
 System.out.println(numberToWord(67));
 System.out.println(numberToWord(999999999));
 }
}

• dreza: Although this is targeting c# and may be slighter different you might find this Code Review question useful - codereview.stackexchange.com/questions/14033/…
• msEmmaMays: This is a duplicate of stackoverflow.com/questions/3911966/…

Answer 1

Some notes:

1. wordForm should be StringBuilder: https://stackoverflow.com/questions/73883/string-vs-stringbuilder Actually, I'd rename it to result too to make it clear that this object stores the result of the method.

2. Instead of the System.out.printlns, use a parametrized unit test:

   import static org.junit.Assert.assertEquals;
   import java.util.Arrays;
   import java.util.Collection;
   import org.junit.Test;
   import org.junit.runner.RunWith;
   import org.junit.runners.Parameterized;
   import org.junit.runners.Parameterized.Parameters;
   @RunWith(value = Parameterized.class)
   public class NumberToWordTest extends NumberToWord {
    private final String expected;
    private final int input;
    public NumberToWordTest(final String expected, final int input) {
    this.expected = expected;
    this.input = input;
    }
    @Parameters
    public static Collection<Object[]> data() {
    final Object[][] data = new Object[][] {
    { "One Hundred Thousand", 100000 },
    { "Nine HundredNinetyNine Thousand Nine HundredNinetyNine", 999999 },
    { "Six HundredSeventyEight Thousand Nine Hundred", 678900 },
    { "Zero", 0 },
    { "One Hundred Thousand Five HundredSixtySeven", 100567 },
    { "FourThousandFive HundredEightyNine", 4589 },
    { "ThreeThousandThree HundredThirtyThree", 3333 },
    { "SixtySeven Thousand Five Hundred", 67500 },
    { "SeventyTwo", 72 },
    { "One HundredSeventyTwo Thousand Three HundredFortySix", 172346 },
    { "Eight HundredNinety Thousand", 890000 },
    { "Six Hundred Thousand Seven Hundred", 600700 },
    { "SixtySeven", 67 },
    { "Nine HundredNinetyNine Million Nine HundredNinetyNine Thousand Nine HundredNinetyNine", 999999999 } };
    return Arrays.asList(data);
    }
    @Test
    public void test() {
    assertEquals(expected, NumberToWord.numberToWord(input));
    }
   }
   

   See: Why not just use System.out.println()? in the JUnit FAQ.

3. The number parameter could be final. It would help readers, because they know that the value does not change later. https://softwareengineering.stackexchange.com/questions/115690/why-declare-final-variables-inside-methods but you can find other questions on Programmers.SE in the topic.

4. According to the Code Conventions for the Java Programming Language

   if statements always use braces {}.

   Omitting them is error-prone.

5. Numbers like 1000000000, 1000000 etc. are magic numbers. You should use named constants.

   private static final int ZERO = 0;
   private static final int TEN = 10;
   private static final int ONE_HUNDRED = 100;
   private static final int ONE_THOUSAND = 1000;
   private static final int TEN_THOUSANDS = 10000;
   private static final int ONE_BILLION = 1000000000;
   private static final int ONE_MILLION = 1000000;
   

6. The method does not handle negative numbers. In this case you should throw an IllegalArgumentException instead of returning an empty string. (See: Effective Java, 2nd edition, Item 38: Check parameters for validity)

7. The reference type of numberMap should be simply Map<Integer, String>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

8. numberMap could be private and unmodifiable. It would prevent accidental modification.

   private static final Map<Integer, String> numberMap = createNumberMap();
   private static Map<Integer, String> createNumberMap() {
    final Map<Integer, String> numberMap = new HashMap<Integer, String>();
    numberMap.put(0, "Zero");
    numberMap.put(1, "One");
    ...
    return Collections.unmodifiableMap(numberMap);
   }
   

   What's the deal with Java's public fields?

9. I'd try to extract out a few methods:

   private static void calc(final StringBuilder result, final int number, 
    final int divisor, final String postfix) {
    final int quotient = number / divisor;
    final int remainder = number % divisor;
    if (quotient != 0) {
    result.append(numberToWord(quotient));
    result.append(" ");
    result.append(postfix);
    }
    if (remainder != 0) {
    result.append(" ");
    result.append(numberToWord(remainder));
    }
   }
   private static void calc2(final StringBuilder result, final int number, 
    final int divisor, final String postfix) {
    final int quotient = number / divisor;
    final int remainder = number % divisor;
    if (quotient != 0) {
    result.append(numberMap.get(quotient));
    result.append(postfix);
    }
    if (remainder != 0) {
    result.append(numberToWord(remainder));
    }
   }
   private static void calc3(final StringBuilder result, final int number, 
    final int divisor) {
    final int quotient = number / divisor;
    final int remainder = number % divisor;
    if (quotient != 0) {
    result.append(numberMap.get(quotient * divisor));
    }
    if (remainder != 0) {
    result.append(numberMap.get(remainder));
    }
   }
   

   It would remove some code duplication. Maybe you can find a better names for them.

   Then, the numberToWord looks like this:

   public static String numberToWord(final int number) {
    if (number < 0) {
    throw new IllegalArgumentException("Number can't be negative: " + number);
    }
    final StringBuilder result = new StringBuilder();
    if (number < ONE_BILLION && number >= ONE_MILLION) {
    calc(result, number, ONE_MILLION, "Million");
    } else if (number < ONE_MILLION && number >= TEN_THOUSANDS) {
    calc(result, number, ONE_THOUSAND, "Thousand");
    } else if (number < TEN_THOUSANDS && number >= ONE_THOUSAND) {
    calc2(result, number, ONE_THOUSAND, "Thousand");
    } else if (number < ONE_THOUSAND && number >= ONE_HUNDRED) {
    calc2(result, number, ONE_HUNDRED, " Hundred");
    } else if (number < ONE_HUNDRED && number >= TEN) {
    calc3(result, number, TEN);
    } else if (number < TEN && number >= ZERO) {
    result.append(numberMap.get(number));
    }
    return result.toString();
   }
   

• Phoenix: palacsint do you think the code is easy to maintain and readable?
• palacsint: @Phoenix: I think it's fine, I didn't have any big problem during the review/refactoring. I'd not consider any of my comment serious.

Answer 2

Here is a sample I played with. Hope it's useful.

I'd consider decomposing this into reusable functions.

package com.bluenoteandroid.experimental.ints;
import static com.google.common.base.Optional.*;
import static com.google.common.base.Preconditions.*;
import static java.lang.Math.*;
import java.util.Arrays;
import java.util.LinkedList;
import com.google.common.annotations.Beta;
import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
public final class Ints {
 public static final int BASE_10 = 10;
 public static final String MINUS = "minus"; 
 public static final ImmutableMap<Integer, String> NUMBERS = ImmutableMap.<Integer, String>builder()
 .put(0,"zero")
 .put(1,"one")
 .put(2,"two")
 .put(3,"three")
 .put(4,"four")
 .put(5,"five")
 .put(6,"six")
 .put(7,"seven")
 .put(8,"eight")
 .put(9,"nine")
 .put(10,"ten")
 .put(11,"eleven")
 .put(12,"twelve")
 .put(13,"thirteen")
 .put(14,"fourteen")
 .put(15,"fifteen")
 .put(16,"sixteen")
 .put(17,"seventeen")
 .put(18,"eighteen")
 .put(19,"nineteen")
 .put(20,"twenty")
 .put(30,"thirty")
 .put(40,"forty")
 .put(50,"fifty")
 .put(60,"sixty")
 .put(70,"seventy")
 .put(80,"eighty")
 .put(90,"ninety")
 .put(100,"hundred")
 .put(1000,"thousand")
 .put(1000000,"million")
 .put(1000000000,"billion")
 .build();
 private Ints() { /* disabled */ }
 public static boolean allZeros(int[] range) {
 for (int n : range) {
 if (n != 0) {
 return false;
 }
 }
 return true;
 }
 /**
 * Counts digits in an integer
 * @param number
 * @return digits count in the number
 */
 public static int digitCount(final int number) {
 return (int) ((number == 0) ? 1 : log10(abs(number)) + 1);
 }
 /**
 * Sums digits in an integer
 * @param number number
 * @return sum of the n's digits
 */
 public static int digitSum(final int number) {
 int _n = abs(number);
 int sum = 0;
 do {
 sum += _n % BASE_10;
 } while ((_n /= BASE_10) > 0);
 return sum;
 }
 /**
 * Gets digit index in an integer, counting from right 
 * @param number number
 * @param index (from right)
 * @return index-th digit from n
 */
 public static int digitFromRight(final int number, final int index) {
 checkElementIndex(index, digitCount(number));
 return (int) ((abs(number) / pow(BASE_10, index)) % BASE_10);
 }
 /**
 * Gets digit of index in an integer, counting from left 
 * @param number
 * @param index (from left)
 * @return index-th digit from n
 */
 public static int digitFromLeft(final int number, final int index) {
 checkElementIndex(index, digitCount(number));
 return digitFromRight(number, digitCount(number) - index - 1);
 }
 /**
 * Converts a number to the array of it's digits
 * @param number number
 * @return array of digits
 */
 public static int[] digits(final int number) {
 return digits(number, digitCount(number));
 }
 /**
 * Converts a number to a reversed array of it's digits
 * @param number number
 * @return reversed array of digits
 */
 public static int[] digitsReversed(final int number) {
 return digitsReversed(number, digitCount(number));
 }
 private static int[] digits(final int number, final int digitCount) {
 final int[] digits = new int[digitCount];
 int _n = abs(number);
 int i = digitCount - 1;
 do {
 digits[i--] = _n % BASE_10;
 } while ((_n /= BASE_10) > 0);
 return digits;
 }
 /**
 * Converts a number to a reversed array of it's digits
 * @param number number
 * @param digitCount digit count
 * @return reversed array of digits
 */
 private static int[] digitsReversed(final int number, final int digitCount) {
 final int[] reversedDigits = new int[digitCount];
 int _n = abs(number);
 int i = 0;
 do {
 reversedDigits[i++] = _n % BASE_10;
 } while ((_n /= BASE_10) > 0);
 return reversedDigits;
 }
 public static int fromDigits(final int[] digits) {
 int n = 0;
 int i = 0;
 do { 
 n += digits[i++];
 if (i >= digits.length) {
 break;
 }
 n *= BASE_10;
 } while (true);
 return n;
 }
 public static int fromDigitsReversed(final int[] reversedDigits) {
 int n = 0;
 int i = reversedDigits.length;
 do { 
 n += reversedDigits[--i];
 if (i <= 0) {
 break;
 }
 n *= BASE_10;
 } while (true);
 return n;
 }
 @Beta
 public static String numberInWords(final int number) {
 Optional<String> name = fromNullable(NUMBERS.get(number));
 if (name.isPresent()) {
 return name.get();
 } else {
 return numberToName(number);
 } 
 }
 private static String numberToName(int number) { 
 final LinkedList<String> words = Lists.newLinkedList();
 final int[] digits = digitsReversed(number); 
 for (int factor = 0; factor < digits.length; factor += 3) {
 final int[] range = Arrays.copyOfRange(digits, factor, factor + 3);
 if (allZeros(range)) {
 continue;
 }
 switch (factor) {
 case 0: /* nothing */
 break;
 case 3: /* thousand */
 words.addFirst(NUMBERS.get((int) pow(BASE_10, 3)));
 break;
 case 6: /* million */
 words.addFirst(NUMBERS.get((int) pow(BASE_10, 6)));
 break;
 case 9: /* billion */
 words.addFirst(NUMBERS.get((int) pow(BASE_10, 9)));
 break;
 default:
 throw new IllegalStateException("Unknown factor: " + factor);
 }
 String part = tripletToWords(range);
 words.addFirst(part);
 }
 if (number < 0) { // negative
 words.addFirst(MINUS);
 }
 return Joiner.on(' ').join(words);
 }
 private static String tripletToWords(final int[] reversedDigits) {
 checkArgument(reversedDigits.length == 3, "This is not a triplet of digits, size: " + reversedDigits.length);
 final int number = fromDigitsReversed(reversedDigits);
 final int[] range = Arrays.copyOfRange(reversedDigits, 0, 2);
 final String dubletWords = dubletToWords(range);
 if (number >= 100) {
 final int thirdDigit = reversedDigits[2];
 final int factor = BASE_10 * BASE_10;
 final String dublet = allZeros(range) ? null : dubletWords;
 return Joiner.on(' ').skipNulls().join(
 NUMBERS.get(thirdDigit),
 NUMBERS.get(factor),
 dublet);
 } else {
 return dubletWords;
 }
 }
 private static String dubletToWords(final int[] reversedDigits) {
 checkArgument(reversedDigits.length == 2, "This is not a dublet of digits, size: " + reversedDigits.length);
 final int number = fromDigitsReversed(reversedDigits);
 Optional<String> name = fromNullable(NUMBERS.get(number));
 if (name.isPresent()) {
 return name.get();
 } else {
 final int firstDigit = reversedDigits[0];
 final int secondDigit = reversedDigits[1];
 final int tens = BASE_10 * secondDigit;
 return Joiner.on('-').join(
 NUMBERS.get(tens),
 NUMBERS.get(firstDigit));
 }
 }
}

With unit tests:

package com.bluenoteandroid.experimental.ints;
import static com.bluenoteandroid.experimental.ints.Ints.*;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.util.Arrays;
import java.util.Collection;
import org.junit.Test;
import org.junit.experimental.categories.Categories;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import org.junit.runners.Suite.SuiteClasses;
import com.bluenoteandroid.experimental.ints.IntsTests.*;
@RunWith(Categories.class)
@SuiteClasses({ 
 CountTest.class, 
 SumTest.class, 
 GetDigitRightTest.class, 
 GetDigitRightPeconditionsTest.class,
 GetDigitLeftTest.class, 
 GetDigitLeftPeconditionsTest.class,
 DigitArrayTest.class,
 FromDigitArrayTest.class,
 ReverseDigitArrayTest.class,
 FromReverseDigitArrayTest.class,
 NumberToWordTest.class
})
public class IntsTests {
 @RunWith(value = Parameterized.class) 
 public static class CountTest {
 private final int input;
 private final int expected;
 public CountTest(final int input, final int expected) {
 this.input = input;
 this.expected = expected;
 }
 @Parameters
 public static Collection<Integer[]> data() {
 return Arrays.asList(new Integer[][] {
 { 0, 1 }, /* input, expected */
 { -1, 1 },
 { 900003245, 9 },
 });
 }
 @Test
 public void shouldCountDigits() {
 assertThat(digitCount(input), is(expected));
 }
 }
 @RunWith(value = Parameterized.class)
 public static class SumTest {
 private final int input;
 private final int expected;
 public SumTest(final int input, final int expected) {
 this.input = input;
 this.expected = expected;
 }
 @Parameters
 public static Collection<Integer[]> data() {
 return Arrays.asList(new Integer[][] {
 { 0, 0 }, /* input, expected */
 { -1, 1 },
 { 1001, 2 },
 { 1234, 1+2+3+4 }
 });
 }
 @Test
 public void shouldSumDigits() {
 assertThat(digitSum(input), is(expected));
 }
 }
 @RunWith(value = Parameterized.class)
 public static class GetDigitRightTest {
 private final int input;
 private final int index;
 private final int expected;
 public GetDigitRightTest(final int input, final int index, final int expected) {
 this.input = input;
 this.index = index;
 this.expected = expected;
 }
 @Parameters
 public static Collection<Integer[]> data() {
 return Arrays.asList(new Integer[][] {
 { 0, 0, 0 }, /* input, expected */
 { -1004, 0, 4 },
 { 1234, 1, 3 },
 { -1234, 2, 2 },
 { 1234, 3, 1 },
 });
 }
 @Test
 public void shouldGetDigits() {
 assertThat(digitFromRight(input, index), is(expected));
 }
 }
 public static class GetDigitRightPeconditionsTest {
 @Test(expected=IndexOutOfBoundsException.class)
 public void shouldNotAllowNegativeIndex() {
 digitFromRight(1234, -1);
 }
 @Test(expected=IndexOutOfBoundsException.class)
 public void shouldNotAllowToBigIndex() {
 digitFromRight(1234, 4);
 }
 }
 @RunWith(value = Parameterized.class)
 public static class GetDigitLeftTest {
 private final int input;
 private final int index;
 private final int expected;
 public GetDigitLeftTest(final int input, final int index, final int expected) {
 this.input = input;
 this.index = index;
 this.expected = expected;
 }
 @Parameters
 public static Collection<Integer[]> data() {
 return Arrays.asList(new Integer[][] {
 { 0, 0, 0 }, /* input, expected */
 { -1004, 0, 1 },
 { 1234, 1, 2 },
 { -1234, 2, 3 },
 { 1234, 3, 4 },
 });
 }
 @Test
 public void shouldGetDigits() {
 assertThat(digitFromLeft(input, index), is(expected));
 }
 }
 public static class GetDigitLeftPeconditionsTest {
 @Test(expected=IndexOutOfBoundsException.class)
 public void shouldNotAllowNegativeIndex() {
 digitFromLeft(1234, -1);
 }
 @Test(expected=IndexOutOfBoundsException.class)
 public void shouldNotAllowToBigIndex() {
 digitFromLeft(1234, 4);
 }
 }
 public static class DigitArrayTest {
 @Test
 public void shouldGetAllDigits() {
 final int[] result = digits(-1234);
 final int[] expected = new int[] {1,2,3,4};
 assertThat(result, is(expected));
 }
 }
 public static class FromDigitArrayTest {
 @Test
 public void shouldConvertDigits() {
 final int result = fromDigits(new int[] {1,2,3,4});
 final int expected = 1234;
 assertThat(result, is(expected));
 }
 }
 public static class ReverseDigitArrayTest {
 @Test
 public void shouldGetAllDigits() {
 final int[] result = digitsReversed(-1234);
 final int[] expected = new int[] {4,3,2,1};
 assertThat(result, is(expected));
 }
 }
 public static class FromReverseDigitArrayTest {
 @Test
 public void shouldConvertDigits() {
 final int result = fromDigitsReversed(new int[] {4,3,2,1});
 final int expected = 1234;
 assertThat(result, is(expected));
 }
 }
 @RunWith(value = Parameterized.class)
 public static class NumberToWordTest {
 private final int input;
 private final String expected;
 public NumberToWordTest(final int input, final String expected) {
 this.input = input;
 this.expected = expected;
 }
 @Parameters
 public static Collection<Object[]> data() {
 return Arrays.asList(new Object[][] {
 { 0, "zero" },
 { 1, "one" },
 { 10, "ten" },
 { 15, "fifteen" },
 { 60, "sixty" },
 { 67, "sixty-seven" },
 { 72, "seventy-two" },
 { 101, "one hundred one" },
 { 205, "two hundred five" },
 { 4589, "four thousand five hundred eighty-nine" },
 { 3333, "three thousand three hundred thirty-three" },
 { 67500, "sixty-seven thousand five hundred" },
 { 100000, "one hundred thousand" },
 { 100567, "one hundred thousand five hundred sixty-seven" },
 { 172346, "one hundred seventy-two thousand three hundred forty-six" },
 { 600700, "six hundred thousand seven hundred" },
 { 678900, "six hundred seventy-eight thousand nine hundred" },
 { 890000, "eight hundred ninety thousand" },
 { 999999, "nine hundred ninety-nine thousand nine hundred ninety-nine" },
 { 999999999, "nine hundred ninety-nine million nine hundred ninety-nine thousand nine hundred ninety-nine" },
 { 1999999999, "one billion nine hundred ninety-nine million nine hundred ninety-nine thousand nine hundred ninety-nine" },
 { -21239, "minus twenty-one thousand two hundred thirty-nine"},
 });
 }
 @Test
 public void test() {
 assertEquals(expected, numberInWords(input));
 }
 }
}