This converts a "numerical number" to a text representation, for example:
convert 1234 --> "One Thousand, Two Hundred Thirty Four"
I'd like general feedback on how it could be make more readable or idiomatic. There are a few specific points I'd like improved as well. The word lookup functions are a little messy. Each uses a different method of looking up the name depending on how the input relates to the desired name. If they could be generalized, it would be much neater. It's also not that efficient. It requires a few reversals, all of which are necessary with my current technique; as far as I can tell.
import Data.Char
import Data.List
-- Global names
places = ["","Thousand","Million","Billion","Trillion","Quadrillion","Quintillion"]
ones = ["One","Two","Three","Four","Five","Six","Seven","Eight","Nine"]
teens = ["Ten","Eleven","Twelve","Thirteen","Fourteen","Fifteen","Sixteen","Seventeen","Eighteen","Nineteen"]
tens = ["A 1 made it to getTens","Hundred","Twenty","Thirty","Forty","Fifty","Sixty","Seventy","Eighty","Ninety"]
-- Word lookup functions
getPlaceName :: Int -> String
getPlaceName place = places !! place
getOnes :: Char -> String
getOnes '0' = ""
getOnes n = let index = (digitToInt n) - 1 in
ones !! index
getTeens :: String -> String
getTeens n = let r = read n :: Int in
head $ [x | (i,x) <- (zip [10..] teens), i == r]
getTens :: Char -> String
getTens '0' = ""
getTens n = let index = (digitToInt n) in
tens !! index
--Takes a string rep. of a number, reverses it, and groups it into 3-groups
-- 12,000 -> ["000", "21"]
cutIntoGroups :: String -> [String]
cutIntoGroups = reverse . cut [] . reverse
where
cut acc [] = acc
cut acc rest = let (next,rest') = splitAt 3 rest in
cut (next : acc) rest'
-- Applies the "rules" to each sub-chunk
applyLocalRules :: String -> String
applyLocalRules revChunk = hund ++ tensNOnes
where
(ones,tens,hunds) = toTriple revChunk '0'
tensNOnes = if tens == '1' then (getTeens [tens,ones])
else (getTens tens) ++ (getOnes ones)
hund = if hunds == '0' then ""
else (getOnes hunds) ++ "Hundred"
-- Succeedes each formatted chunk with the place name
applyRules :: [String] -> [String]
applyRules = reverse . map (\(i,g) -> let pName = getPlaceName i in
applyLocalRules g ++ pName) . zip [0..]
toTriple :: [a] -> a -> (a,a,a)
toTriple [] def = (def,def,def)
toTriple xs def = case xs of
[x,y,z] -> (x,y,z)
[x,y] -> (x,y,def)
[x] -> (x,def,def)
_ -> error "List greater then 3"
addCommasAndConcat :: [String] -> String
addCommasAndConcat = concat . map (\chunk -> chunk ++ ",")
-- Adds spaces before words
addSpaces :: String -> String
addSpaces [] = []
addSpaces left = (h : pre) ++ " " ++ addSpaces post
where
(h:t) = left
(pre,post) = break isUpper t
-- Loosely stolen from SO
trim :: String -> String
trim = let t = reverse . dropWhile (not . isAlpha) in
t . t
-- Main function
convert :: Int -> String
convert = trim . addSpaces . addCommasAndConcat
. applyRules . cutIntoGroups . show
It should be noted that this is very unsafe in its current form, so I'm only looking for comments regarding the algorithm itself (it crashes ungracefully on negative numbers, non-numbers, and too-large of numbers). I think I'm going to use this module to test out the Either Monad later to fix its safety.
1 Answer 1
I added the following test code to investigate. It does an exhaustive check from 1 to 999 and then a few more cases.
import Data.Monoid
import Control.Monad
import Test.Hspec
specTest n w = it ("converts " <> show n) $ convert n `shouldBe` w
oneToNine = words "One Two Three Four Five Six Seven Eight Nine"
zeroToNineEndings = "" : map ("-"<>) oneToNine
oneToNinetyNine = oneToNine
<> words "Ten Eleven Twelve Thirteen Fourteen Fifteen Sixteen Seventeen Eighteen Nineteen"
<> [decade <> ending | decade <- words "Twenty Thirty Forty Fifty Sixty Seventy Eighty Ninety", ending <- zeroToNineEndings]
main = hspec $ do
zipWithM_ specTest [1..]
$ oneToNinetyNine
<> mconcat
[[d <> " Hundred"] <> map ((d <> " Hundred ") <>) oneToNinetyNine
| d <- oneToNine
]
specTest 1000 "One Thousand and "
specTest 1001 "One Thousand and One"
specTest 1234 "One Thousand and Two Hundred Thirty-Four"
specTest 1234567 "One Million, Two Hundred Thirty-Four Thousand and Five Hundred Sixty-Seven"
Note that 1000 converts to 'One Thousand and ' which is probably wrong. Also, your original question claims 1234 converts to 'One Thousand, Two Hundred Thirty Four' but in fact it comes out as 'One Thousand and Two Hundred Thirty-Four'.
I am not sure your rules for adding 'and' and commas in are correct, however I'm from the UK where we typically put an 'and' between the hundreds and tens (e.g. 'two hundred and thirty-four') and I'm aware that this isn't how things are done in the US. I'd recommend adding some more test cases as I've shown above to make sure you're getting things as you expect.
The code seems idiomatically ok, although the mixture of CamelCase and use_of_underscores is a little jarring. hlint offers some good advice on the use of idiom and reports this, a few unnecessary parentheses and $s and identifies that concat . intersperse can be written as intercalate. Nothing major.