patternMinor
One, two, three, … Un, deux, trois, …, maxBound::Int
Viewed 0 times
threeintonetwomaxbounddeuxtrois
Problem
I wrote a numbers-to-words converter for English, then tried to adapt the code to work for French.
I'm wondering mainly whether any simplifications are possible.
In English
English is relatively straightforward:
En français
I've tried to stick to "standard" French, using the style guide from Le Figaro and these rules for large numbers.
Relative to English, complications include:
I'm wondering mainly whether any simplifications are possible.
In English
English is relatively straightforward:
english :: Int -> String
english n
| n == 0 = "zero"
| n < 0 = "negative " ++ (english (-n))
| otherwise = kScale 0 $ map wordsTo1k $ splitk $ littleEndianDigits n
where
littleEndianDigits n
| n < 0 = error "Negative"
| n < 10 = [n]
| otherwise = (n `mod` 10) : (littleEndianDigits (n `div` 10))
splitk [] = []
splitk (i:[]) = [(0, 0, i)]
splitk (i:x:[]) = [(0, x, i)]
splitk (i:x:c:n) = (c, x, i) : (splitk n)
wordsTo1k (0, 0, i) = digits !! i
wordsTo1k (0, x, 0) = tens !! x
wordsTo1k (0, 1, i) = teens !! i
wordsTo1k (0, x, i) = (wordsTo1k (0, x, 0)) ++ "-" ++ (wordsTo1k (0, 0, i))
wordsTo1k (c, 0, 0) = (digits !! c) ++ " hundred"
wordsTo1k (c, x, i) = (wordsTo1k (c, 0, 0)) ++ " " ++ (wordsTo1k (0, x, i))
kScale s [] = []
kScale s ("zero":gs) = kScale (s + 1) gs
kScale s (g:gs) = unwords' [ kScale (s + 1) gs, g, kScales !! s ]
unwords' (a:[]) = a
unwords' (a:[""]) = a
unwords' ([]:b) = unwords' b
unwords' (a:b) = a ++ " " ++ unwords' b
digits = ["zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine"]
tens = [error "zero", "ten", "twenty", "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety"]
teens = [error "ten", "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"]
kScales = [[], "thousand", "million", "billion", "trillion", "quadrillion", "quintillion"]En français
I've tried to stick to "standard" French, using the style guide from Le Figaro and these rules for large numbers.
Relative to English, complications include:
- Numbe
Solution
I would move all this functions to the global level:
While it is true that these functions are related to
Non-standard pattern-matching
Is non standard, I suggest
That is more readable.
Use the built-ins
You define your own unwords but there already exists one. Using it may cause extraneous spaces at the edges of the words, but
And then:
Now you can safely delete your home-made
Always write explicit types
The Haskell compiler can infer (deduce) types for you. But I encourage you to write explicit typing all the time, as it:
Use everyday names whenever possible
Do not give names differing in one char only
If you make a typo and switch one for the other debugging will be a nightmare. Chose an equally descriptive synonym.
Avoid abbreviations as often as possible
The meaning of
splitk [] = []
splitk (i:[]) = [(0, 0, i)]
splitk (i:x:[]) = [(0, x, i)]
splitk (i:x:c:n) = (c, x, i) : (splitk n)
wordsTo1k (0, 0, i) = digits !! i
wordsTo1k (0, x, 0) = tens !! x
wordsTo1k (0, 1, i) = teens !! i
wordsTo1k (0, x, i) = (wordsTo1k (0, x, 0)) ++ "-" ++ (wordsTo1k (0, 0, i))
wordsTo1k (c, 0, 0) = (digits !! c) ++ " hundred"
wordsTo1k (c, x, i) = (wordsTo1k (c, 0, 0)) ++ " " ++ (wordsTo1k (0, x, i))
kScale s [] = []
kScale s ("zero":gs) = kScale (s + 1) gs
kScale s (g:gs) = unwords' [ kScale (s + 1) gs, g, kScales !! s ]
unwords' (a:[]) = a
unwords' (a:[""]) = a
unwords' ([]:b) = unwords' b
unwords' (a:b) = a ++ " " ++ unwords' bWhile it is true that these functions are related to
numbers_to_words, putting them at global level increases testability.Non-standard pattern-matching
splitk (i:[]) = [(0, 0, i)]
splitk (i:x:[]) = [(0, x, i)]Is non standard, I suggest
splitk [i] = [(0, 0, i)]
splitk [i, x] = [(0, x, i)]That is more readable.
Use the built-ins
You define your own unwords but there already exists one. Using it may cause extraneous spaces at the edges of the words, but
trim is all it takes to remove those, like this:trim :: String -> String
trim = f . f
where f = reverse . dropWhile isSpaceAnd then:
kScale s (g:gs) = trim $ unwords [ kScale (s + 1) gs, g, kScales !! s ]Now you can safely delete your home-made
unwords' and profit from the code shortening.Always write explicit types
The Haskell compiler can infer (deduce) types for you. But I encourage you to write explicit typing all the time, as it:
- Increases efficiency (inferred types are more generic and more slow).
- Takes very little time (it took me less than a minute to write types for your functions).
- Checks that you and the compiler think the same about the same piece of code.
- Simplifies reading, mainly with curried definitions.
Use everyday names whenever possible
littleEndianDigits vaguely recalls me something about bit stored in a computer to represent integers, something like right to left or left to right. I was not sure about the order of the digits until I executed it. Instead if you name it reversedDigits people will understand that for example reversedDigits 123 == [3, 2, 1] in the blink of an eye.Do not give names differing in one char only
tens and teens
kScale and kScalesIf you make a typo and switch one for the other debugging will be a nightmare. Chose an equally descriptive synonym.
Avoid abbreviations as often as possible
The meaning of
kScales is not immediately clear as it is not the meaning of splitk. Using Thousands in full is sure more typing, but I think think that nobody would argue that it increases readibility.Code Snippets
splitk [] = []
splitk (i:[]) = [(0, 0, i)]
splitk (i:x:[]) = [(0, x, i)]
splitk (i:x:c:n) = (c, x, i) : (splitk n)
wordsTo1k (0, 0, i) = digits !! i
wordsTo1k (0, x, 0) = tens !! x
wordsTo1k (0, 1, i) = teens !! i
wordsTo1k (0, x, i) = (wordsTo1k (0, x, 0)) ++ "-" ++ (wordsTo1k (0, 0, i))
wordsTo1k (c, 0, 0) = (digits !! c) ++ " hundred"
wordsTo1k (c, x, i) = (wordsTo1k (c, 0, 0)) ++ " " ++ (wordsTo1k (0, x, i))
kScale s [] = []
kScale s ("zero":gs) = kScale (s + 1) gs
kScale s (g:gs) = unwords' [ kScale (s + 1) gs, g, kScales !! s ]
unwords' (a:[]) = a
unwords' (a:[""]) = a
unwords' ([]:b) = unwords' b
unwords' (a:b) = a ++ " " ++ unwords' bsplitk (i:[]) = [(0, 0, i)]
splitk (i:x:[]) = [(0, x, i)]splitk [i] = [(0, 0, i)]
splitk [i, x] = [(0, x, i)]trim :: String -> String
trim = f . f
where f = reverse . dropWhile isSpacekScale s (g:gs) = trim $ unwords [ kScale (s + 1) gs, g, kScales !! s ]Context
StackExchange Code Review Q#104302, answer score: 9
Revisions (0)
No revisions yet.