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

Finding word acronyms for telephone numbers

Submitted by: @import:stackexchange-codereview··
0
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.

{-# 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: 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 toRegex is so general that it almost useless. I think you actually meant Integer -> Maybe String;



  • in parseList you first scanning list for Nothing and then catMaybes scanning it again to remove Nothings. It is possible to do this in single pass by using sequence.



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.