snippetMinor
Convert a number to text
Viewed 0 times
converttextnumber
Problem
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) 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
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) 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
Solution
I added the following test code to investigate. It does an exhaustive check from 1 to 999 and then a few more cases.
Note that 1000 converts to
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.
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 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.Code Snippets
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"Context
StackExchange Code Review Q#67453, answer score: 3
Revisions (0)
No revisions yet.