I'm currently working on a simple command line application in Haskell and I have three functions that are quite repetitive.

The three functions:

getAllFilmsByDirector :: [Film] -> String -> String
getAllFilmsByDirector filmsDb directorName = unlines [name ++ ", " ++ director ++ ", " ++ show year ++ ", " ++ show (calculateRatings likes dislikes)|(name, director, year, likes, dislikes) <- filmsDb, director == directorName]

getAllFilmsWithHighRatings :: [Film] -> String
getAllFilmsWithHighRatings filmsDb = unlines [name ++ ", " ++ director ++ ", " ++ show year ++ ", " ++ show (calculateRatings likes dislikes)|(name, director, year, likes, dislikes) <- filmsDb, (calculateRatings likes dislikes) >= 75]

getAllFilmsByDirectorAvgRating :: [Film] -> String -> Float
getAllFilmsByDirectorAvgRating filmsDb directorName = roundTo (calculateAverageRating [calculateRatings likes dislikes|(name, director, year, likes, dislikes) <- filmsDb, director == directorName]) 1

Film Type

type Film = (String, String, Int, [String], [String])

testDatabase :: [Film]
testDatabase = [("Blade Runner", "Ridley Scott", 1982, ["Zoe", "Heidi", "Jo", "Kate", "Emma", "Liz", "Dave"], ["Sam", "Olga", "Tim"]), ("The Fly", "David Cronenberg", 1986, ["Garry", "Dave", "Zoe"], ["Kevin", "Emma", "Heidi", "Jo", "Kate"]), ("Body Of Lies", "Ridley Scott", 2008, ["Garry", "Dave"], ["Bill", "Olga", "Tim", "Zoe", "Paula"]), ("Avatar", "James Cameron", 2009, ["Dave", "Amy", "Liz"], ["Olga", "Tim", "Zoe", "Paula"]), ("Titanic", "James Cameron", 1997, ["Zoe", "Emma", "Paula", "Liz", "Olga", "Dave"], ["Sam", "Wally", "Kate"]), ("The Departed", "Martin Scorsese", 2006, ["Wally", "Liz", "Kevin", "Tim", "Emma"], ["Olga", "Dave", "Kate", "Zoe"]), ("Aliens", "Ridley Scott", 1986, ["Dave", "Garry", "Liz", "Sam", "Wally", "Kate", "Zoe"], ["Tim", "Emma", "Jo", "Olga"]), ("Kingdom Of Heaven", "Ridley Scott", 2005, ["Jo", "Wally", "Emma"], ["Tim", "Garry", "Ian", "Neal"]), ("Alien: Covenant", "Ridley Scott", 2017, ["Kevin", "Tim"], ["Emma", "Jo", "Liz"]), ("E.T. The Extra-Terrestrial", "Steven Spielberg", 1982, ["Dave", "Amy", "Garry", "Ian", "Neal"], ["Jenny", "Kate", "Emma", "Olga"]), ("Bridge of Spies", "Steven Spielberg", 2015, ["Wally", "Sam", "Dave", "Neal"], ["Bill", "Garry", "Ian", "Kate"]), ("Jaws", "Steven Spielberg", 1975, ["Jenny", "Emma", "Bill", "Neal"], ["Sam", "Ian", "Kate"]), ("The Martian", "Ridley Scott", 2015, ["Wally", "Sam", "Dave", "Jo", "Jenny", "Kate", "Emma", "Olga"], ["Ian", "Neal", "Tim", "Liz"]), ("The BFG", "Steven Spielberg", 2016, ["Sam", "Wally", "Dave", "Jo", "Kate"], ["Neal"]), ("The Shawshank Redemption", "Frank Darabont", 1994, ["Dave", "Amy", "Bill", "Garry", "Ian", "Neal", "Kate", "Jenny", "Zoe", "Heidi"], ["Jo"]), ("Gladiator", "Ridley Scott", 2000, ["Olga", "Neal", "Kate", "Garry"], ["Heidi", "Bill", "Sam", "Zoe"]), ("The Green Mile", "Frank Darabont", 1999, ["Kevin", "Tim", "Emma", "Heidi"], ["Kate", "Jenny", "Zoe"]), ("True Lies", "James Cameron", 1994, ["Sam", "Dave"], ["Emma", "Olga", "Jenny", "Zoe"]), ("Super 8", "J J Abrams", 2011, ["Kevin", "Tim", "Emma", "Olga", "Heidi"], ["Wally", "Dave", "Jenny", "Zoe"]), ("Minority Report", "Steven Spielberg", 2002, ["Kevin", "Kate", "Tim", "Emma", "Jenny", "Zoe"], ["Olga", "Heidi"]), ("War Horse", "Steven Spielberg", 2011, ["Garry", "Bill", "Olga", "Jo", "Wally", "Emma", "Tim", "Kate", "Zoe"], ["Heidi", "Jenny", "Sam"]), ("Silence", "Martin Scorsese", 2016, ["Wally", "Emma", "Tim", "Heidi", "Bill", "Jo"], ["Dave", "Olga"]), ("The Terminal", "Steven Spielberg", 2004, ["Kate", "Dave", "Jo", "Wally", "Emma"], ["Heidi"]), ("Star Wars: The Force Awakens", "J J Abrams", 2015, ["Emma", "Wally", "Zoe", "Kate", "Bill", "Dave", "Liz"], ["Olga", "Jo", "Neal"]), ("Hugo", "Martin Scorsese", 2011, ["Wally", "Sam"], ["Kate", "Bill", "Dave"])]

roundTo

roundTo x n = (fromIntegral (round (x * 10^n))) / 10^n

calculateRatings

calculateRatings :: [String] -> [String] -> Int
calculateRatings likes dislikes = round(fromIntegral (length likes) / fromIntegral (length likes + length dislikes) * 100)

calculateAvgRating

calculateAverageRating :: [Int] -> Float
calculateAverageRating rating = fromIntegral (round(fromIntegral ((foldr (+) 0 rating)) / fromIntegral (length rating) * 100)) / 100

I'm fairly new to Haskell and its syntax but I know I should probably make use of the getAllFilmsByDirector in the getAllFilmsByDirectorAvgRating however because the getAllFilmsByDirectorAvgRating takes a Film type rather than a string, I'm not too sure about the best way to modify the code to make use of the getFilmsByDirector function.

I'm also using very similar list comprehensions in each of the three and if anyone has any ideas on how I can make these less repetitive that would be amazing.

Don't repeat yourself

Your code suffers from being WET. Your Film's formatting function is repeated. So let us get rid of that first:

prettyFilm :: Film -> String
prettyFilm (name, director, year, likes, dislikes) =
 name ++ ", " ++ director ++ ", " ++ show year ++ ", " ++ show rating
 where rating = calculateRatings likes dislikes

Now getAllFilmsByDirector and getAllFilmsWithHighRatings are slightly easier to read:

getAllFilmsByDirector :: [Film] -> String -> String
getAllFilmsByDirector filmsDb directorName =
 unlines [prettyFilm film | film@(_, director, _, _, _) <- filmsDb
 , director == directorName]

getAllFilmsWithHighRatings :: [Film] -> String
getAllFilmsWithHighRatings filmsDb = 
 unlines [prettyFilm film | film@(_, _, _, likes, dislikes) <- filmsDb
 , calculateRatings likes dislikes >= 75]

However, that still looks like there is of duplicate code: in the end both functions filter the filmsDb and then print all films. If we had getFilmsBy :: (Film -> Bool) -> [Film] -> [Film] we could write:

getAllFilmsByDirector :: [Film] -> String -> String
getAllFilmsByDirector filmsDb directorName = unlines . map prettyFilm . getFilmsBy p $ filmsDb 
 where
 p (_, _, director, _, _) = director == directorName

getAllFilmsWithHighRatings :: [Film] -> String
getAllFilmsWithHighRatings filmsDb = unlines . map prettyFilm . getFilmsBy p $ filmsDb 
 where
 p (_, _, _, likes, dislikes) = calculateRatings likes dislikes >= 75

Luckily, getFilmsBy = filter.

Now that we got rid of the list comprehensions, we can see that we can still improve our functions if we extract unlines . map prettyFilm . filter:

showAllFilmsBy :: (Film -> Bool) -> [Film] -> String
showAllFilmsBy p = unlines . map prettyFilm . filter p 

getAllFilmsByDirector :: [Film] -> String -> String
getAllFilmsByDirector filmsDb directorName = showAllFilmsBy p filmsDb
 where
 p (_, _, director, _, _) = director == directorName

getAllFilmsWithHighRatings :: [Film] -> String
getAllFilmsWithHighRatings filmsDb = showAllFilmsBy p filmsDb
 where
 p (_, _, _, likes, dislikes) = calculateRatings likes dislikes >= 75

Other review items

• Some of your parentheses are superfluous, for example those in roundTo. Try to get rid of those.


• Almost all your functions have type signatures (good!), except for roundTo (bad). Try to add type signatures to all top-level bindings.


• Unless you really need to use Double instead of Float.


• Try to stay in the original type Film as long as possible. You can always go from Film to String, but the inverse isn't true. That's why you couldn't reuse your code.

Try to put configuration-like parameters to the left and data-like parameters to the right, it composes better.

genericLength condenses much conversion.

ImplicitParams lets me improvise a query language so I don't need to unpack the tuple everywhere.

You already wrote roundTo and then you didn't use it.

-# LANGUAGE ImplicitParams #-

getAllFilmsByDirector :: String -> [Film] -> String
getAllFilmsByDirector director = unlines . selectwhere ?pretty (?director == director)

getAllFilmsWithHighRatings :: [Film] -> String
getAllFilmsWithHighRatings = unlines . selectwhere ?pretty (?rating >= 0.75)

getAllFilmsByDirectorAvgRating :: String -> [Film] -> Float
getAllFilmsByDirectorAvgRating director
 = roundTo 1 . liftA2 (/) sum genericLength . selectwhere ?rating (?director == director)

-- selectwhere :: Query a -> Query Bool -> [Film] -> [a]
selectwhere select cond = map (exec select) . filter (exec cond) where
 -- exec :: Query a -> Film -> a
 exec query (name, director, year, likes, dislikes) = let
 ?rating = roundTo 2 $ genericLength likes / (genericLength likes + genericLength dislikes)
 ?pretty = name ++ ", " ++ director ++ ", " ++ show year ++ ", " ++ show ?ratings
 ?director = director
 in query

roundTo n x = fromIntegral (round (x * 10^n)) / 10^n