patternMinor
Finding word acronyms for telephone numbers
Viewed 0 times
numberstelephonewordacronymsforfinding
Problem
Having read most of Learn You a Haskell as well as parts of Real World Haskell, I decided to embark on an easy project to see if I can apply the Haskell that I've learned.
I'm interested in any suggestions for making my code more efficient/idiomatic.
{-# LANGUAGE OverloadedStrings #-}
import Data.Maybe
import Text.Regex.TDFA
import qualified Data.ByteString.Char8 as B
main = do
dictionary B.ByteString
outputMessage Nothing = "Some input was incorrect. Make sure no letters or numbers with 0 or 1 are being used."
outputMessage (Just []) = "Sorry. No matches were found."
outputMessage (Just x) = "Matching words: \n" `B.append` ( B.unlines x)
getMatches _ Nothing = Nothing
getMatches _ (Just "") = Just []
getMatches words (Just pattern) = if (words == B.empty)
then error "Was not able to read dictionary file."
else Just (getAllTextMatches (words =~ (B.pack ("\\b" ++ pattern ++ "\\b")) ))
toPattern = parseList . map toRegex . readNumber
parseList list
| Nothing `elem` list = Nothing
| otherwise = Just (concat . catMaybes $ list)
toRegex :: (Data.String.IsString a1, Num a, Eq a) => a -> Maybe a1
toRegex 2 = Just "(a|b|c)"
toRegex 3 = Just "(d|e|f)"
toRegex 4 = Just "(g|h|i)"
toRegex 5 = Just "(j|k|l)"
toRegex 6 = Just "(m|n|o)"
toRegex 7 = Just "(p|q|r|s)"
toRegex 8 = Just "(t|u|v)"
toRegex 9 = Just "(w|x|y|z)"
toRegex _ = Nothing
readNumber :: [Char] -> [Integer]
readNumber = map $ read . (:"")I'm interested in any suggestions for making my code more efficient/idiomatic.
Solution
This is pretty nice code.
I have two main concerns. The first one is about handling errors.
You are mixing tree approaches:
It's better to stick to
The second one is about dividing your program in meaningful functions each having single responsibility. Such functions are simpler and you can more easily compose them.
For example,
It is better to split error handling from data processing (in this case by lifting dictionary check to
Another one is
You can skip this as you check for invalid symbols in
Some other remarks:
Here is my take:
I have two main concerns. The first one is about handling errors.
You are mixing tree approaches:
Maybe, throwing explicit error, throwing implicit error (as in readNumber).It's better to stick to
Maybe or Either — they are explicit and you can handle them in pure code.The second one is about dividing your program in meaningful functions each having single responsibility. Such functions are simpler and you can more easily compose them.
For example,
getMatches checks if dictionary is not empty, adds \b to pattern and matches pattern.It is better to split error handling from data processing (in this case by lifting dictionary check to
main).Another one is
readNumber: it parses string into list of digits and implicitly checks for nondigits (throwing useless 'no parse' error).You can skip this as you check for invalid symbols in
toRegex.Some other remarks:
- the type of
toRegexis so general that it almost useless. I think you actually meantInteger -> Maybe String;
- in
parseListyou first scanning list forNothingand thencatMaybesscanning it again to removeNothings. It is possible to do this in single pass by usingsequence.
Here is my take:
{-# LANGUAGE OverloadedStrings #-}
import Text.Regex.TDFA
import qualified Data.ByteString.Char8 as B
import Data.Monoid ((<>))
import Control.Monad (when)
import System.Exit (die)
-- `die` was introduced in the latest base, you can replace it with
-- die err = putStrLn err >> exitFailure
main = do
dictionary
die "Some input was incorrect. Make sure no letters or numbers with 0 or 1 are being used."
Just pattern -> case getAllTextMatches $ dictionary =~ pattern of
[] -> B.putStrLn $ "Sorry. No matches were found."
res -> B.putStrLn $ "Matching words: \n" <> B.unlines res
toPattern :: String -> Maybe B.ByteString
toPattern str = do
parts B.concat parts <> "\\b"
toRegex :: Char -> Maybe B.ByteString
toRegex = flip lookup
[('2', "(a|b|c)") ,('3', "(d|e|f)")
,('4', "(g|h|i)") ,('5', "(j|k|l)")
,('6', "(m|n|o)") ,('7', "(p|q|r|s)")
,('8', "(t|u|v)") ,('9', "(w|x|y|z)")
]Code Snippets
{-# LANGUAGE OverloadedStrings #-}
import Text.Regex.TDFA
import qualified Data.ByteString.Char8 as B
import Data.Monoid ((<>))
import Control.Monad (when)
import System.Exit (die)
-- `die` was introduced in the latest base, you can replace it with
-- die err = putStrLn err >> exitFailure
main = do
dictionary <- B.readFile "words.txt"
when (dictionary == "") $ die "Your dictionary is empty."
putStrLn "What is your phone number?"
phoneNumber <- getLine
case toPattern phoneNumber of
Nothing ->
die "Some input was incorrect. Make sure no letters or numbers with 0 or 1 are being used."
Just pattern -> case getAllTextMatches $ dictionary =~ pattern of
[] -> B.putStrLn $ "Sorry. No matches were found."
res -> B.putStrLn $ "Matching words: \n" <> B.unlines res
toPattern :: String -> Maybe B.ByteString
toPattern str = do
parts <- sequence $ map toRegex str
return $ "\\b" <> B.concat parts <> "\\b"
toRegex :: Char -> Maybe B.ByteString
toRegex = flip lookup
[('2', "(a|b|c)") ,('3', "(d|e|f)")
,('4', "(g|h|i)") ,('5', "(j|k|l)")
,('6', "(m|n|o)") ,('7', "(p|q|r|s)")
,('8', "(t|u|v)") ,('9', "(w|x|y|z)")
]Context
StackExchange Code Review Q#94439, answer score: 2
Revisions (0)
No revisions yet.