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

Converting to Roman numerals

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
convertingromannumerals

Problem

As an excercise to "Haskell: The Craft of Functional Programming", I created some code to convert an Integer to Roman numerals.

This is the first version and I think I'm probably not doing it in the "Haskell way" since my background is in imperative programming.

Any advice on how to improve this? Sometimes I think I'm missing a obvious way to do it in just one function.

convMap = [(1000,"M"), (900,"CM"), (500,"D"), (400,"CD"), (100,"C"),
           (90,"XC"), (50,"L"), (40,"XL"), (10,"X"), (9,"IX"), (5,"V"),
           (4,"IV"), (1,"I")]

toRoman :: Integer -> String
toRoman x 
  | x == 0 = "N"
  | otherwise = (snd . numToSubtract $ x) ++ toRoman'(nextNum)
      where nextNum = x - (fst. numToSubtract $ x)

-- Auxiliary function just so we treat 0 differently
-- (avoids 3 == "IIIN" if not used)
toRoman' :: Integer -> String
toRoman' x 
  | x == 0 = ""
  | x > 0 = (snd . numToSubtract $ x) ++ toRoman'(nextNum)
      where nextNum = x - (fst. numToSubtract $ x)

-- Returns which item in the convMap should be subtracted
numToSubtract :: Integer -> (Integer, String)
numToSubtract x = head (filter (lessThan x) convMap)

-- Filter function to work on the tuples in convMap
lessThan :: Integer -> (Integer, String) -> Bool
lessThan n (a, b)
  | a <= n = True
  | otherwise = False

Solution

Booleans are normal values:

lessThan n (a, b)
  | a <= n = True
  | otherwise = False


Simply write:

lessThan n (a, _) = a <= n


Use pattern matching:

toRoman' :: Integer -> String
toRoman' x 
  | x == 0 = ""
  | x > 0 = (snd . numToSubtract $ x) ++ toRoman'(nextNum)
      where nextNum = x - (fst. numToSubtract $ x)


If you want to take fst and snd of the same tuple, think about pattern matching.
Minor point: brackets in toRoman'(nextNum) are redundant, simply write toRoman' nextNum.

toRoman' :: Integer -> String
toRoman' x
  | x == 0 = ""
  | x > 0 = b ++ toRoman' (x - a)
      where (a, b) = numToSubtract x


Remove duplication:

toRoman and toRoman' contain duplicate code. In fact toRoman is only used to treat specially 0, so can be changed:

toRoman :: Integer -> String
toRoman 0 = "N"
toRoman x = toRoman' x


Move or remove utility functions:

lessThan and numToSubtract are used only once and don't seem to be generally useful, maybe inline them:

toRoman' :: Integer -> String
toRoman' x
  | x == 0 = ""
  | x > 0 = b ++ toRoman' (x - a)
      where (a, b) = head $ filter (\(a,_) -> a <= x) convMap


As Dan Burton commented, you can write also:

where (a, b) = head $ filter ((<= x) . fst) convMap


Final code

convMap = [(1000,"M"), (900,"CM"), (500,"D"), (400,"CD"), (100,"C"),
           (90,"XC"), (50,"L"), (40,"XL"), (10,"X"), (9,"IX"), (5,"V"),
           (4,"IV"), (1,"I")]

-- Auxiliary function just so we treat 0 differently
-- (avoids 3 == "IIIN" if not used)
toRoman :: Integer -> String
toRoman 0 = "N"
toRoman x = toRoman' x

toRoman' :: Integer -> String
toRoman' x 
  | x == 0 = ""
  | x > 0 = b ++ toRoman' (x - a)
      where (a, b) = head $ filter ((<= x) . fst) convMap


Fold

If you stare a little at the code, it turns out you each entry from convMap only once. First you check how many M's you need. Then CM's. Then D's. And so on. Using "filter" function always restarts search from beginning. In fact, toRoman can be written as a fold!

import Data.List (genericReplicate)

convMap = [(1000,"M"), (900,"CM"), (500,"D"), (400,"CD"), (100,"C"),
           (90,"XC"), (50,"L"), (40,"XL"), (10,"X"), (9,"IX"), (5,"V"),
           (4,"IV"), (1,"I")]

toRoman :: Integer -> String
toRoman 0 = "N"
toRoman x | x > 0 = snd $ foldl f (x,[]) convMap
  where f (n,s) (rn, rs) = (l, s ++ concat (genericReplicate k rs))
              where (k,l) = divMod n rn


Going crazy with abstractions

Erik Hilton's nice answer gives a solution using unfoldr. So you can write the code using fold and unfold, and these are two ways of looking at it. I believe that combination of fold and unfold is called "paramorphism" or "apomorphism" or something like that...

Code Snippets

lessThan n (a, b)
  | a <= n = True
  | otherwise = False
lessThan n (a, _) = a <= n
toRoman' :: Integer -> String
toRoman' x 
  | x == 0 = ""
  | x > 0 = (snd . numToSubtract $ x) ++ toRoman'(nextNum)
      where nextNum = x - (fst. numToSubtract $ x)
toRoman' :: Integer -> String
toRoman' x
  | x == 0 = ""
  | x > 0 = b ++ toRoman' (x - a)
      where (a, b) = numToSubtract x
toRoman :: Integer -> String
toRoman 0 = "N"
toRoman x = toRoman' x

Context

StackExchange Code Review Q#5550, answer score: 14

Revisions (0)

No revisions yet.