5
\$\begingroup\$

I've written this code in Haskell to convert a number into its written form. It feels ok but at the same time I feel like it can probably be improved on a lot. I would appreciate help in improving it.

nums :: Int -> String
nums x = case x of
    0 -> "zero"
    1 -> "one"
    2 -> "two"
    3 -> "three"
    4 -> "four"
    5 -> "five"
    6 -> "six"
    7 -> "seven"
    8 -> "eight"
    9 -> "nine"
    10 -> "ten"
    11 -> "eleven"
    12 -> "twelve"
    13 -> "thirteen"
    14 -> "fourteen"
    15 -> "fifteen"
    16 -> "sixteen"
    17 -> "seventeen"
    18 -> "eighteen"
    19 -> "nineteen"
    20 -> "twenty"
    30 -> "thirty"
    40 -> "forty"
    50 -> "fifty"
    60 -> "sixty"
    70 -> "seventy"
    80 -> "eighty"
    90 -> "ninety"
    100 -> "hundred"
    _ -> ""

bigNums :: Int -> String
bigNums x =
    case x of
        1 -> "thousand"
        2 -> "million"
        3 -> "billion"
        4 -> "trillion"
        5 -> "quadrillion"
        6 -> "quintillion"
        7 -> "sextillion"
        8 -> "septillion"
        9 -> "octillion"
        10 -> "nonillion"

isEmpty :: String -> Bool
isEmpty "" = True
isEmpty s = False

numToEnglish :: String -> Int -> String
numToEnglish start num
    | len > 3 =
        let exponent = ((len-1) `div` 3) * 3
            powerNum = bigNums (exponent `div` 3)
        in
            if powerNum == ""
                then ""
            else
                let numHead = read $ take (len-exponent) stringNum
                    numTail = read $ drop (len-exponent) stringNum
                    numStart = numToEnglish "" numHead ++ " " ++ powerNum
                in startSpace ++ numToEnglish numStart numTail
    | len == 3 = numToEnglish (startSpace ++ nums firstNum ++ " " ++ nums 100) intTail
    | len == 2 =
        let tens = nums (firstNum * 10)
            includeDash = if not $ isEmpty tens then "-" else ""
        in
        if firstNum == 1 
            then startSpace ++ nums num
        else
            if intTail == 0
                then startSpace ++ tens
            else startSpace ++ tens ++ includeDash ++ nums intTail
    | len == 1 =
        if start == ""
            then numHead
        else if firstNum == 0
            then start
        else start ++ " " ++ numHead
    | otherwise = ""
    where
        stringNum = show num
        len = length stringNum
        firstNum = read [head stringNum] :: Int
        numHead = if firstNum > 1 || isEmpty start then nums firstNum else ""
        intTail = read (drop 1 stringNum) :: Int
        startSpace = if isEmpty start then start else start ++ " "


convert :: Int -> String
convert = numToEnglish ""
\$\endgroup\$
5
\$\begingroup\$

I think the primary issue with your current implementation is that it's attempting to do math on strings. For example,

let numHead = read $ take (len-exponent) stringNum
    numTail = read $ drop (len-exponent) stringNum

can be expressed with a much simpler divMod usage. Replacing your numeric strings with numbers will probably lead you to a much simpler and cleaner implementation right out of the gate. Much of your code can remain similar, but numToEnglish would likely be drastically simplified.

Another specific point I would suggest addressing is that nums currently has an unclear responsibility. It does not always convert the number to an English string, and this makes it somewhat harder to understand its usage. I recommend breaking more responsibility into these smaller functions, and having one function do all the work to convert numbers < 100. My implementation (below) also creates another function to deal with multiples of ten, which are mixed together with numbers < 20 in your implementation.

While you certainly could use a function Int -> String for the order of magnitude (what you called bigNums) you can see in my implementation that I think using a list produces a nicer solution, especially if you break the number into parts for each order of magnitude first (as I did).

More minor nitpicks:

You explicitly annotated your functions as taking Ints. The maximum value this algorithm could apply to does not fit into an Int, whose maximum value is about 2^63, or a bit over 9 quintillion. I would suggest using Integral a => a -> String so numbers as large as 999 nonillion can actually be passed to convert.

Your isEmpty function is equivalent to null, from the prelude.

If any invalid inputs are given, your functions will produce non-exhaustive pattern errors or fail silently. The former may be a matter of style, but convert should probably fail quickly if it does not know how to convert the provided number.

Here's my implementation, feel free to let me know if you have any questions:

import Data.List (unfoldr, intercalate)
import Data.Tuple (swap)

-- Convert an integer in [0, 100) to an English number.
-- Example:
-- `convertSmall 64 == "sixty-four"`
convertSmall :: Integral a => a -> String
convertSmall x = case x of
    0 -> ""
    1 -> "one"
    2 -> "two"
    3 -> "three"
    4 -> "four"
    5 -> "five"
    6 -> "six"
    7 -> "seven"
    8 -> "eight"
    9 -> "nine"
    10 -> "ten"
    11 -> "eleven"
    12 -> "twelve"
    13 -> "thirteen"
    14 -> "fourteen"
    15 -> "fifteen"
    16 -> "sixteen"
    17 -> "seventeen"
    18 -> "eighteen"
    19 -> "nineteen"
    x | x < 100 && x >= 20 -> case x `divMod` 10 of
        (tens, 0) -> convertTens (tens * 10)
        (tens, ones) -> convertTens (tens * 10) ++ "-" ++ convertSmall ones
    _ -> error "convertSmall can only be applied to integers in [0, 100)"

-- Converts a multiple of ten in [20, 90] to an English number.
-- Example:
-- `convertTens 30 == "thirty"`
convertTens :: Integral a => a -> String
convertTens x = case x of
    20 -> "twenty"
    30 -> "thirty"
    40 -> "forty"
    50 -> "fifty"
    60 -> "sixty"
    70 -> "seventy"
    80 -> "eighty"
    90 -> "ninety"
    _ -> error "convertTens can only be applied to multiples of 10 in [20, 90]"

-- Converts a number in [0, 1000) to an English number.
-- Example:
-- `convertHundreds 289 == "two hundred eighty-nine"`
convertHundreds :: Integral a => a -> String
convertHundreds x | x >= 0 && x < 1000 = case x `divMod` 100 of
    (0, 0) -> ""
    (0, rest) -> convertSmall rest
    (hundreds, 0) -> convertSmall hundreds ++ " hundred"
    (hundreds, rest) -> convertSmall hundreds ++ " hundred " ++ convertSmall rest
convertHundreds _ = error "convertHundreds can only be applied to integers in [0, 1000)"

-- The names of orders of magnitude
orders :: [String]
orders =
    [ ""
    , " thousand"
    , " million"
    , " billion"
    , " trillion"
    , " quadrillion"
    , " quintillion"
    , " sextillion"
    , " septillion"
    , " octillion"
    , " nonillion"
    ]

-- Get the parts of each order of magnitude.
-- Example:
-- `orderParts 1391290 == [290, 391, 13]
orderParts :: Integral a => a -> [a]
orderParts x = unfoldr getParts x
    where
        getParts :: Integral a => a -> Maybe (a, a)
        getParts 0 = Nothing
        getParts x = Just $ swap $ x `divMod` 1000

-- Convert a number and an order into an English number.
-- Example:
-- `convertOrder " million" 734 == "seven hundred thirty-four million"`
convertOrder :: Integral a => String -> a -> String
convertOrder _ 0 = ""
convertOrder orderName x = convertHundreds x ++ orderName

-- Convert a number into an English number.
-- Example:
-- `convert 782301 == "seven hundred eighty-two thousand three hundred one"`
convert :: Integral a => a -> String
convert 0 = "zero"
convert x | (fromIntegral x :: Integer) < 10 ^ 33
    = intercalate " "
    $ reverse
    $ filter (not . null)
    $ zipWith convertOrder orders
    $ orderParts x
convert _ = error "convert can only be applied to integers in [0, 10^33)"
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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