Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Following along with some previous questions:

I wanted to answer the original question with a different algorithm, but was not able to get the VB.net code to work. I see @nhgrif had the same idea as me, and proposed a Swift solution.

This is the algorithm I would use, but in Java.

Like other questions, I am looking for possible improvements, or suggestions for other aspects that may make this more robust, and more usable.

public class IntToText {

    private static final String[] SCALES = {"", "thousand", "million", "billion", "trillion", "quadrillion", "quintillion", "sextillion"};
    private static final String[] SUBTWENTY = {"zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
                                               "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"};
    private static final String[] DECADES = {"zero", "ten", "twenty", "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety"};

    /**
     * Convert any value from 0 to 999 inclusive, to a string.
     * @param value The value to convert.
     * @param and whether to use the word 'and' in the output.
     * @return a String representation of the value.
     */
    private static final String tripleAsText(int value, boolean and) {
        if (value < 0 || value >= 1000) {
            throw new IllegalArgumentException("Illegal triple-value " + value);
        }

        if (value < SUBTWENTY.length) {
            return SUBTWENTY[value];
        }

        int subhun = value % 100;
        int hun = value / 100;
        StringBuilder sb = new StringBuilder(50);
        if (hun > 0) {
            sb.append(SUBTWENTY[hun]).append(" hundred");
        }
        if (subhun > 0) {
            if (hun > 0) {
                sb.append(and ? " and " : " ");
            }
            if (subhun < SUBTWENTY.length) {
                sb.append(SUBTWENTY[subhun]);
            } else {
                int tens = subhun / 10;
                int units = subhun % 10;
                if (tens > 0) {
                    sb.append(DECADES[tens]);
                }
                if (units > 0) {
                    sb.append(" ").append(SUBTWENTY[units]);
                }
            }
        }

        return sb.toString();
    }

    /**
     * Convert any long input value to a text representation 
     * @param value The value to convert
     * @param useand true if you want to use the word 'and' in the text (eleven thousand and thirteen)
     * @param negname
     * @return
     */
    public static final String asText(long value, boolean useand, String negname) {
        if (value == 0) {
            return SUBTWENTY[0];
        }

        // break the value down in to sets of three digits (thousands).
        int[] thous = new int[SCALES.length];
        boolean neg = value < 0;
        // do not make negative numbers positive, to handle Long.MIN_VALUE
        int scale = 0;
        while (value != 0) {
            // use abs to convert thousand-groups to positive, if needed.
            thous[scale] = Math.abs((int)(value % 1000));
            value /= 1000;
            scale++;
        }

        StringBuilder sb = new StringBuilder(scale * 40);
        if (neg) {
            sb.append(negname).append(" ");
        }
        boolean first = true;
        while (--scale > 0) {
            if (!first) {
                sb.append(", ");
            }
            first = false;
            if (thous[scale] > 0) {
                sb.append(tripleAsText(thous[scale], useand)).append(" ").append(SCALES[scale]);
            }

        }

        if (!first && useand && thous[0] != 0) {
            sb.append(" and ");
        }
        sb.append(tripleAsText(thous[0], useand));

        return sb.toString();
    }

    public static void main(String[] args) {
        System.out.printf("%15d %s%n", Integer.MIN_VALUE, asText(Integer.MIN_VALUE, true, "minus"));
        System.out.printf("%15d %s%n", Integer.MAX_VALUE, asText(Integer.MAX_VALUE, true, "minus"));
        System.out.printf("%15d %s%n", 0, asText(0, true, "minus"));
        System.out.printf("%15d %s%n", Long.MIN_VALUE, asText(Long.MIN_VALUE, true, "minus"));
        System.out.printf("%15d %s%n", Long.MAX_VALUE, asText(Long.MAX_VALUE, true, "minus"));
    }
}

The program produces the output (added 'x' to avoid bulletted formatting problem):

x
x     -2147483648 minus two billion, one hundred and forty seven million, four hundred and eighty three thousand and six hundred and forty eight
x      2147483647 two billion, one hundred and forty seven million, four hundred and eighty three thousand and six hundred and forty seven
x               0 zero
x -9223372036854775808 minus nine quintillion, two hundred and twenty three quadrillion, three hundred and seventy two trillion, thirty six billion, eight hundred and fifty four million, seven hundred and seventy five thousand and eight hundred and eight
x 9223372036854775807 nine quintillion, two hundred and twenty three quadrillion, three hundred and seventy two trillion, thirty six billion, eight hundred and fifty four million, seven hundred and seventy five thousand and eight hundred and seven
share|improve this question

3 Answers 3

up vote 10 down vote accepted

I would define DECADES[0] as "" instead of "zero" so that the if (tens>0) can be removed. Notice that DECADES[0] was never used and its value "zero" was a repetition with respect to SUBTWENTY[0].

Also there is a repetition in the creation of the "subhundred" part of the number.

In fact you can safely remove this part of code:

    if (value < SUBTWENTY.length) {
        return SUBTWENTY[value];
    }

which is anyway correctly handled by the code following.

Looking carefully you see that there is a problem with the handling of "zero". Try your code with 1000 to see that it doesn't work.

Notice however that the tripleAsText should return the emptystring when value is 0, because "zero" should be considered as a special case and trapped in the main function (notice that "zero" is never used in the spelling of numbers apart from zero itself).

So your function could be simplified as:

private static final String[] SCALES = {"", "thousand", "million", "billion", "trillion", "quadrillion", "quintillion", "sextillion"};
private static final String[] SUBTWENTY = {"", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
                                           "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"};
private static final String[] DECADES = {"", "ten", "twenty", "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety"};

/**
 * Convert any value from 0 to 999 inclusive, to a string.
 * @param value The value to convert.
 * @param and whether to use the word 'and' in the output.
 * @return a String representation of the value.
 */
private static final String tripleAsText(int value, boolean and) {
    int subhun = value % 100;
    int hun = value / 100;
    StringBuilder sb = new StringBuilder(50);
    if (hun > 0) {
        sb.append(SUBTWENTY[hun]).append(" hundred ");
        if (subhun > 0 && and) {
            sb.append("and ");
        }
    }
    if (subhun < SUBTWENTY.length) {
        sb.append(SUBTWENTY[subhun]);
    } else {
        int tens = subhun / 10;
        int units = subhun % 10;
        sb.append(DECADES[tens]);
        if (units>0) {
          sb.append(" ");
        }
        sb.append(SUBTWENTY[units]);
    }

    return sb.toString();
}
share|improve this answer
    
Excellent feedback, especially noticing the 1000 bug. Thanks –  rolfl Aug 3 at 12:34

First of all, in tripleAsText the first check for value < SUBTWENTY.length is entirely superfluous. It does give improved performance in those cases where the value is < 20, but it also causes your code to fail the DRY (Don't Repeat Yourself) test.

Also, the methods are arguably a little longer than necessary. I prefer to pull out any piece that does one specific thing into a function of its own. This reduces the length and complexity of the original function and creates a self-documenting label describing what that piece of functionality does.

For example, the part of tripleAsText that does the 0-99 part could be extracted to a doubleAsText method:

private static final String doubleAsText(int value) {
        if (subhun < SUBTWENTY.length) {
            sb.append(SUBTWENTY[subhun]);
        } else {
            int tens = subhun / 10;
            int units = subhun % 10;

            sb.append(DECADES[tens]);
            if (units > 0) {
                sb.append(" ").append(SUBTWENTY[units]);
            }
        }
    }

Notice I used Emanuele Paolini's suggestion to eliminate the if (tens > 0). Also, since this code is only used internally, the check to make sure that value is in the correct range should not be needed.

share|improve this answer

I am going to Nitpick your English here

The program produces the output (added 'x' to avoid bulletted formatting problem):

x
x     -2147483648 minus two billion, one hundred and forty seven million, four hundred and eighty three thousand and six hundred and forty eight
x      2147483647 two billion, one hundred and forty seven million, four hundred and eighty three thousand and six hundred and forty seven
x               0 zero
x -9223372036854775808 minus nine quintillion, two hundred and twenty three quadrillion, three hundred and seventy two trillion, thirty six billion, eight hundred and fifty four million, seven hundred and seventy five thousand and eight hundred and eight
x 9223372036854775807 nine quintillion, two hundred and twenty three quadrillion, three hundred and seventy two trillion, thirty six billion, eight hundred and fifty four million, seven hundred and seventy five thousand and eight hundred and seven

It shouldn't be "minus" for a negative number, it should be "negative"

Also numbers greater than 20 and less than 100 should be hyphenated so the numbers should be ouput like

-2147483648 negative two billion, one hundred and forty-seven million, four hundred and eighty-three thousand, six hundred and forty-eight

Numbers in English

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.