8
\$\begingroup\$

This is a simple mixed drink calculator written in Haskell.

There are two input files. The drinks file contains a list of simply formatted recipes for mixed drinks:

screwdriver:vodka,orange juice
white russian:vodka,coffee liqueur,cream
greyhound:gin,grapefruit juice
...

The ingredients file contains a line-separated list of presently available ingredients:

vodka
gin
rum
orange juice
cranberry juice
grapefruit juice
...

The calculator parses the drinks file and the ingredients file, determines which mixed drinks can be created with the available ingredients, and pretty-prints the results to screen:

Screwdriver: Vodka, Orange juice
Greyhound: Gin, Grapefruit juice
...

It's a simple program, but I want to improve my code style and make my Haskell more idiomatic.

import Text.ParserCombinators.Parsec
import Data.Char
import Data.List
type Drink = (DrinkName, [Ingredient])
type DrinkName = String
type Ingredient = String
-- parsing the file of drink recipes 
drinksFile ∷ GenParser Char st [Drink]
drinksFile = endBy drink (char '\n')
-- parsing each drink recipe 
drink ∷ GenParser Char st Drink
drink = do first ← drinkName
 next ← recipe
 return (first, next)
-- parsing the drink name 
drinkName ∷ GenParser Char st DrinkName
drinkName = do name ← many (noneOf ":")
 sep ← many (char ':')
 return name
-- parsing the recipe into a list of ingredients 
recipe ∷ GenParser Char st [Ingredient]
recipe = sepBy (many (noneOf ",\n")) (char ',')
-- returns true iff the drink can be made with the given ingredients 
canMake ∷ [Ingredient] → Drink → Bool
canMake ingredients drink = all (flip elem ingredients) recipe
 where recipe = snd drink
-- returns the drinks that can be made with the given ingredients 
filterByIngredients ∷ [Drink] → [Ingredient] → [Drink]
filterByIngredients drinks ingredients = filter (canMake ingredients) drinks
-- pretty prints a drink with its recipe 
printDrink ∷ Drink → String
printDrink (drinkName, recipe) = intercalate " " (map capitalize (words drinkName)) ++
 ": " ++
 intercalate ", " (map capitalize recipe)
 where capitalize (x : xs) = toUpper x : xs
main = do drinks ← readFile "drinks"
 ingredients ← readFile "ingredients"
 case parse drinksFile "drinksFile" drinks of
 Left error → do putStrLn "Error parsing drinks file."
 Right parsedDrinks → mapM_ (putStrLn ∘ printDrink) 
 (filterByIngredients parsedDrinks 
 (lines ingredients))
asked Sep 14, 2013 at 23:52
\$\endgroup\$
2
  • \$\begingroup\$ Can you either remove unicode syntax or upload the file to hpaste or other place where I can easily get bit-exact downloads from? Somehow I get parse errors. \$\endgroup\$ Commented Oct 16, 2013 at 17:11
  • 1
    \$\begingroup\$ +1 Yes, making many mixed drinks is very idiomatic for Haskell. Now we just need a comprehensive drinks.txt file... \$\endgroup\$ Commented Apr 27, 2014 at 20:47

1 Answer 1

2
\$\begingroup\$

Good News:

Architecturally, your code looks very well designed. I can't think of any language features that would significantly improve your code's architecture. That leaves only function semantics and readability.

Advise:

This advise isn't specific to idiomatic Haskell but rather general code readability.

Your current main method does a little too much. the contents of main should have a high level of abstraction. Consider abstracting the work done at the end of your main function:

main = do drinks <- readFile "drinks"
 ingredients <- readFile "ingredients"
 putStrLn $ 
 case parse drinksFile "drinksFile" drinks of
 Left error -> "Error parsing drinks file."
 Right parsedDrinks -> showPossibleDrinks parsedDrinks (lines ingredients)
showPossibleDrinks :: [Drink] -> [Ingredient] -> String
showPossibleDrinks drinks ingredients = showDrinks $ filterByIngredients drinks ingredients
showDrinks :: [Drink] -> String
showDrinks = unlines . map printDrink

Here we have abstracted the process of printing the possible drinks into two sub methods. This provides greater readability and also allows us to change the return type of case from IO to String. Hence the putStrLn outside the case statement.


By changing the argument order of a few functions, we can move the functions towards pointfree form:

canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (flip elem ingredients) . snd
filterByIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterByIngredients ingredients = filter (canMake ingredients)
main = do drinks <- readFile "drinks"
 ingredients <- readFile "ingredients"
 putStrLn $
 case parse drinksFile "drinksFile" drinks of
 Left error -> "Error parsing drinks file."
 Right parsedDrinks -> showPossibleDrinks (lines ingredients) parsedDrinks
showPossibleDrinks :: [Ingredient] -> [Drink] -> String
showPossibleDrinks = showDrinks . (filterByIngredients ingredients)

Given the succinctness of your methods, moving towards pointfree form is quite readable and more idiomatic.


If we import the <$> operator from Control.Applicative after transposing the function's arguments we can make filterByIngredients completely pointfree:

filterByIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterByIngredients = filter <$> canMake

You can make the following equivalence substitutions:

(flip elem ingredients) ==> (`elem` ingredients)
intercalate " " ==> unwords

Giving you:

canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (`elem` ingredients) . snd
printDrink :: Drink -> String
printDrink (drinkName, recipe) = unwords (map capitalize (words drinkName)) ++
 ": " ++
 intercalate ", " (map capitalize recipe)
 where capitalize (x : xs) = toUpper x : xs

All together we have:

import Control.Applicative ((<$>))
import Data.Char
import Data.List
import Text.ParserCombinators.Parsec
type Drink = (DrinkName, [Ingredient])
type DrinkName = String
type Ingredient = String
-- parsing the file of drink recipes
drinksFile :: GenParser Char st [Drink]
drinksFile = endBy drink (char '\n')
-- parsing each drink recipe
drink :: GenParser Char st Drink
drink = do first <- drinkName
 next <- recipe
 return (first, next)
-- parsing the drink name
drinkName :: GenParser Char st DrinkName
drinkName = do name <- many (noneOf ":")
 sep <- many (char ':')
 return name
-- parsing the recipe into a list of ingredients
recipe :: GenParser Char st [Ingredient]
recipe = sepBy (many (noneOf ",\n")) (char ',')
canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (`elem` ingredients) . snd
filterbyIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterbyIngredients = filter <$> canMake
-- pretty prints a drink with its recipe
printDrink :: Drink -> String
printDrink (drinkName, recipe) = unwords (map capitalize (words drinkName)) ++
 ": " ++
 intercalate ", " (map capitalize recipe)
 where capitalize (x : xs) = toUpper x : xs
showPossibleDrinks :: [Ingredient] -> [Drink] -> String
showPossibleDrinks ingredients = showDrinks . filterbyIngredients ingredients
showDrinks :: [Drink] -> String
showDrinks = unlines . map printDrink
main = do drinks <- readFile "drinks"
 ingredients <- readFile "ingredients"
 putStrLn $ 
 case parse drinksFile "drinksFile" drinks of
 Left error -> "Error parsing drinks file."
 Right parsedDrinks -> showPossibleDrinks (lines ingredients) parsedDrinks
answered Apr 27, 2014 at 22:16
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.