Spelling Java long values to human readable text in English

Question

Now, I have extended my program taking some answers in the previous iteration into account:

package com.github.coderodde.fun;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;

public class LongSpeller {

    private static final class BigNumber {
        private final long size;
        private final String name;

        private BigNumber(long size, String name) {
            this.size = size;
            this.name = name;
        }

        public long getSize() {
            return size;
        }

        public String getName() {
            return name;
        }
    }

    private static final class Pair {
        private final String name;
        private final Long number;

        Pair(String name, Long number) {
            this.name = name;
            this.number = number;
        }
    }

    private static final BigNumber[] BIG_NUMBERS = {
        new BigNumber(1L, ""),
        new BigNumber(1_000L, "thousand"),
        new BigNumber(1_000_000L, "million"),
        new BigNumber(1_000_000_000L, "billion"),
        new BigNumber(1_000_000_000_000L, "trillion"),
        new BigNumber(1_000_000_000_000_000L, "quadrillion"),
        new BigNumber(1_000_000_000_000_000_000L, "quintillion"),
    };

    private static final Map<Long, String> WORDS = new HashMap<>();

    static {
        WORDS.put(1L, "one");
        WORDS.put(2L, "two");
        WORDS.put(3L, "three");
        WORDS.put(4L, "four");
        WORDS.put(5L, "five");
        WORDS.put(6L, "six");
        WORDS.put(7L, "seven");
        WORDS.put(8L, "eight");
        WORDS.put(9L, "nine");
        WORDS.put(10L, "ten");
        WORDS.put(11L, "eleven");
        WORDS.put(12L, "twelve");
        WORDS.put(13L, "thirteen");
        WORDS.put(14L, "fourteen");
        WORDS.put(15L, "fifteen");
        WORDS.put(16L, "sixteen");
        WORDS.put(17L, "seventeen");
        WORDS.put(18L, "eighteen");
        WORDS.put(19L, "nineteen");
        WORDS.put(20L, "twenty");
        WORDS.put(30L, "thirty");
        WORDS.put(40L, "fourty");
        WORDS.put(50L, "fifty");
        WORDS.put(60L, "sixty");
        WORDS.put(70L, "seventy");
        WORDS.put(80L, "eighty");
        WORDS.put(90L, "ninety");
    }

    /**
     * Spells an input long value as text. For example, for input 2048, the 
     * return value will be "two thousand forty-eight".
     * 
     * @param num the value to spell.
     * @return text representation of {@code num}.
     */
    public static String spell(long num) {
        if (num == Long.MIN_VALUE) {
            return "(not supported)";
        } else if (num == 0) {
            return "zero";
        }

        StringBuilder sb = new StringBuilder();

        if (num < 0) {
            sb.append("minus ");
            num = -num;
        }

        int orderOfMagnitude = 0;
        Deque<Pair> parts = new ArrayDeque<>();

        do {
            parts.addLast(
                    new Pair(BIG_NUMBERS[orderOfMagnitude++].getName(), 
                             num % 1_000L));

            num /= 1_000L;
        } while (num > 0);

        while (parts.size() > 1) {
            Pair pair = parts.removeLast();
            convertHundredsImpl(
                    pair.number, 
                    sb).append(" ")
                       .append(pair.name)
                       .append(" ");
        }

        convertHundredsImpl(parts.removeLast().number, sb);
        return sb.toString().trim();
    }

    // Converts a num in range [0, 999] to the human readable string:
    private static StringBuilder
         convertHundredsImpl(long num, StringBuilder sb) {

        long hundreds = num / 100;
        num -= 100 * hundreds;

        if (hundreds > 0) {
            sb.append(WORDS.get(hundreds));
            sb.append(" hundred ");
        }

        if (num > 0) {
            String tensString = WORDS.get(num);

            if (tensString == null) {
                long units = num % 10;
                num -= units;
                sb.append(WORDS.get(num));
                sb.append("-");
                sb.append(WORDS.get(units));
            } else {
                sb.append(tensString);
            }
        }

        return sb;
    }

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);

        while (scanner.hasNextLong()) {
            long num = scanner.nextLong();
            String s = spell(num);
            System.out.println(s);
        }
    }
}

Critique request

So what do you think? Am I getting it right?

Answer 1

Some thoughts

Looking up names for numbers

Your WORDS Map looks rather like a "code smell" - otherwise you'd have been able to find a much better name for it.

In my suggestions for your previous post along these lines, I had at least two reasons to split the map into two arrays :-

1. It contains two similar, but not equivalent types of names, which are going to be handled differently, so they make more sense stored separately
2. Using arrays for the look-up of names for numbers is natural, and more efficient than using a Map - leaving aside the overhead of creating the map, the boxing and unboxing of the numbers is a significant overhead. I'd sooner worry about the overhead of using a Map vs the efficiency of "do...while" vs "while".

addLast/removeLast

If you are doing stack operations, they can be more naturally expressed using a Deque implementation (such as java.util.ArrayDeque) - push() and pop(). Yes, I know they are equivalent operations, but push and pop better convey your intentions to readers of the code.

However I'm far from convinced of the need for this processing which seems to be an artifact of the ordering of the BIG_NUMBERS table. In English we spell numbers from left to right, so it seems much more natural for your code to process them that way.

Pair and BigNumber

Apart from the order of the fields, and the difference between long and Long (did Pair need to contain Long?), these classes look very similar - couldn't you just have one? Of course, if you order the BIG_NUMBER table descending in size, you don't even need the Pair class.

convertHundredsImpl

We usually use the "Impl" suffix for the implementation of an abstract "thing" - why use it here?

Overall

This just looks bigger and more complex than it needs to be.

Answer 2

I'd say that the guard clause can be a little refined to look more like a guard clause:

        if (num == Long.MIN_VALUE) return "(not supported)";
        if (num == 0) return "zero";

Now it's immediately clear what those statements are there for, to catch edge cases. I didn't have to think much longer in your version, but I did have to think a bit more to see that those are guard clauses. It may also just be my notion of writing guard clauses, but I would ditch the else in any case, since we're throwing or returning inside the guards anyways and the else isn't needed.

Answer 3

I think your methods spell and convertHundredsImpl are too long and have too many nestings. And by too many nestings I mean anything more than one level of indention inside a function is too much.

I like to use these general rules of thumb:

• Is the method longer than 5 lines? -> Extract some of it.
• Does the method have more than one level of indention? E.g. nested ifs or loops? -> Extract until it only has one level indention.

For more info on why to make methods smaller, Long Method is a great resource (and the rest of the website as well).

For more in depth info on how to make code more readable, you can view my recent post on codereview meta about it

Answer 4

I think the Pair class is not necessary, you could just use a LinkedHashMap and call LinkedHashMap#clear() afterwards. Also, if you are using Java 17, you can use Records to reduce the boilerplate in your BigNumber class.