HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

One, two, three, … Un, deux, trois, …, maxBound::Int

Submitted by: @import:stackexchange-codereview··
0
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:

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:

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


While 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 isSpace


And 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 kScales


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 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' b
splitk (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 isSpace
kScale 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.