patternModerate
Am I thinking functionally in these simple Haskell functions?
Viewed 0 times
simplethesethinkinghaskellfunctionsfunctionally
Problem
I'm learning Haskell using the University of Pennsylvania's online materials. I'm a few lessons in and was looking for some feedback about whether I'm thinking functionally enough or porting over my Python background inappropriately.
Below are my answers to the problems set out in lesson three (these were someone's homework once but not any more, and were never mine!). Is my code below making any rookie mistakes for someone coming from another paradigm to functional?
``
| otherwise = ' '
getData :: (Eq a, Ord a) => [a] -> Int -> [a]
getData xs n = [fst x | x = n]
histRow :: [Int] -> String
histRow xs = map (spaceOrStar xs) [0..9]
buildHist :: [Int] -> String
buildHist xs = intercalate "\n" . map (histRow) $ map (getData xs) . reverse $ [1..maxInstances xs]
histogram :: [Int] -> String
histogram xs = buildHist xs ++ "\n==========\n0123456789\n"
main :: IO()
main = do
print $ skips "ABCD"
print $ skips "hello!"
print $ skips [1]
print $ skips [True, False]
Below are my answers to the problems set out in lesson three (these were someone's homework once but not any more, and were never mine!). Is my code below making any rookie mistakes for someone coming from another paradigm to functional?
``
import Data.List
-- Exercise 1
-- Take a list xs and an integer n, and return a list of every nth element of xs
everyNth :: [a] -> Int -> [a]
everyNth xs n = [snd x | x [[a]]
skips xs = map (everyNth xs) [1..length xs]
-- Exercise 2
-- Take a list of integers and produce a list of local maxima
triples :: [a] -> [[a]]
triples (x:xs)
| length (x:xs) [a] -> Bool
isMiddleMax (x:y:z:[]) = x z
localMaxima :: [Integer] -> [Integer]
localMaxima xs = map maximum . filter isMiddleMax $ triples xs
-- Exercise 3
-- Take a list of integers 0-9 and return a histogram
groupSort :: (Eq a, Ord a) => [a] -> [[a]]
groupSort xs = group $ sort xs
countInstance :: (Eq a, Ord a) => [a] -> (a, Int)
countInstance xs = (head xs, length xs)
countInstances :: (Eq a, Ord a) => [a] -> [(a, Int)]
countInstances xs = map (countInstance) $ groupSort xs
maxInstances :: [Int] -> Int
maxInstances xs = maximum $ map (snd) $ countInstances xs
spaceOrStar :: [Int] -> Int-> Char
spaceOrStar xs y
| y elem` xs = '*'| otherwise = ' '
getData :: (Eq a, Ord a) => [a] -> Int -> [a]
getData xs n = [fst x | x = n]
histRow :: [Int] -> String
histRow xs = map (spaceOrStar xs) [0..9]
buildHist :: [Int] -> String
buildHist xs = intercalate "\n" . map (histRow) $ map (getData xs) . reverse $ [1..maxInstances xs]
histogram :: [Int] -> String
histogram xs = buildHist xs ++ "\n==========\n0123456789\n"
main :: IO()
main = do
print $ skips "ABCD"
print $ skips "hello!"
print $ skips [1]
print $ skips [True, False]
Solution
Overall it's pretty nice. I like that you've learned how to use
This makes it easier to refactor later, if you want to do something else than print, write them to different files, all kinds of stuff. The lisp-ish approach of code-as-data is quite useful when enumerating cases like you did here.
Spooky! Use
Dunno if you know, but there exists a thing called "alternate name capture"; it's written like so:
If you consider this an overkill, why not simply change the clause to
The parens around
Oh, and also, in the second case; multiple
This expresses it in a truly functional way, and makes it susceptible to eta-reduction!:
This is called point-free (not to be confused with pointless
I'll add more if I notice any more areas for improvement.
$; it really improves readability in my opinion. One thing that immediately caught my attention though was the chain of prints. Haskell can do better! You can for example put those in a list, and mapM_ print over it.let fns =
[ skips "ABCD"
, skips "hello!"
, skips [1]
, skips [True, False]
, skips ([] :: [Int])
, localMaxima [2,9,5,6,1]
, localMaxima [2,3,4,1,5]
, localMaxima [1,2,3,4,5]
]
mapM_ print fnsThis makes it easier to refactor later, if you want to do something else than print, write them to different files, all kinds of stuff. The lisp-ish approach of code-as-data is quite useful when enumerating cases like you did here.
isMiddleMax (x:y:z:[]) = x zSpooky! Use
-Wall to get a proper warning for that one: incomplete patterns in function .... You might want to consider adding a meaningful error message:isMiddleMax _ = error "Only works on 3-element lists!"triples (x:xs)
| length (x:xs) < 3 = []Dunno if you know, but there exists a thing called "alternate name capture"; it's written like so:
triples (x:xs @ allXs)
| length allXs < 3 = []If you consider this an overkill, why not simply change the clause to
length xs < 2? I don't like the same pattern repeated for some reason.countInstances xs = map (countInstance) $ groupSort xs
maxInstances xs = maximum $ map (snd) $ countInstances xsThe parens around
countInstance and snd are unnecessary (it's not terrible to leave them, just pointing that out). HLint is a useful tool that can provide such hints for you!Oh, and also, in the second case; multiple
$ are a bit less readable; consider this instead:maxInstances xs = maximum . map (snd) . countInstances $ xsThis expresses it in a truly functional way, and makes it susceptible to eta-reduction!:
maxInstances = maximum . map (snd) . countInstancesThis is called point-free (not to be confused with pointless
:)) notation, and makes it extremely clear that the function is indeed a composition of other functions ((.) is Haskell's composition operator).I'll add more if I notice any more areas for improvement.
Code Snippets
let fns =
[ skips "ABCD"
, skips "hello!"
, skips [1]
, skips [True, False]
, skips ([] :: [Int])
, localMaxima [2,9,5,6,1]
, localMaxima [2,3,4,1,5]
, localMaxima [1,2,3,4,5]
]
mapM_ print fnsisMiddleMax (x:y:z:[]) = x < y && y > zisMiddleMax _ = error "Only works on 3-element lists!"triples (x:xs)
| length (x:xs) < 3 = []triples (x:xs @ allXs)
| length allXs < 3 = []Context
StackExchange Code Review Q#78802, answer score: 15
Revisions (0)
No revisions yet.