1
\$\begingroup\$
isLineTerminator c = c == '\r' || c == '\n'
leadWords [] = []
leadWords cs =
 let (pre, suf) = break isLineTerminator cs
 in (head (words pre)) : case suf of 
 ('\r':'\n':rest) -> leadWords rest
 ('\r':rest) -> leadWords rest
 ('\n':rest) -> leadWords rest
 _ -> []
fixLines :: String -> String
fixLines input = unlines (leadWords input)
interactWith function inputFile outputFile = do
 input <- readFile inputFile
 writeFile outputFile (function input)
main = mainWidth myFunction
 where mainWidth function = do
 args <- getArgs
 case args of 
 [input, output] -> interactWith function input output
 _ -> putStrLn "error: exactly two arguments needed"
 myFunction = fixLines

Much of this program is written by the author of the book on Haskell that I'm reading, so I assume that much of it is good. However, my part is in altering the leadWords function. Does it look like a:

  1. readable, elegant function?
  2. efficient and optimized function?
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Oct 28, 2018 at 17:10
\$\endgroup\$
2
  • \$\begingroup\$ Why did you alter it? What was wrong with the original? \$\endgroup\$ Commented Oct 28, 2018 at 18:20
  • \$\begingroup\$ The original did not extract the first word. it just converted the lines from Windows formate to Linux format. \$\endgroup\$ Commented Oct 29, 2018 at 1:22

1 Answer 1

2
\$\begingroup\$

Errors

The code block above is an example where your program will fail. Any empty line will lead to an empty list in words "" and therefore to head [], which calls error.

Standard library

That being said, there are some other issues with your code. You use unlines in fixLines, yet leadWords is essentially lines, although it also uses head . words internally. If we rewrite leadWords with map, we end up with

leadWords = map (head . words) . lines

which immediately shows the possible error on empty lines. Still, we can fix this if we write another function:

firstWord :: String -> Maybe String
firstWord xs = case words xs of
 (x : _) -> Just x
 _ -> Nothing

Now we can use mapMaybe from Data.Maybe to write leadWords:

leadWords :: String -> [String]
leadWords = mapMaybe firstWord . lines

Type signatures

Other than that, you should really add type signatures to your top-level functions. Only fixLines has one, and its not clear why that one got special handling.

Code complexity

main is overly complicated, too. We can just inline mainWidth, as its local binding doesn't yield any more comfort:

main :: IO ()
main = do
 args <- getArgs
 case args of 
 [input, output] -> interactWith fixLines input output
 _ -> putStrLn "error: exactly two arguments needed"

Alternatively, we can provide mainWith as top level function and use that in main.

Complete code

If we apply all remarks given above, we end up with something similar to the following code:

import Data.Maybe (mapMaybe)
mainWith :: (String -> String) -> IO ()
mainWith f = do
 args <- getArgs
 case args of 
 [input, output] -> interactWith f input output
 _ -> putStrLn "error: exactly two arguments needed"
fixLines :: String -> String
fixLines = unlines . leadWords
main :: IO ()
main = mainWith fixLines
-- for completeness
leadWords :: String -> [String]
leadWords = mapMaybe firstWord . lines
firstWord :: String -> Maybe String
firstWord xs = case words xs of
 (x : _) -> Just x
 _ -> Nothing
-- That's just personal preference, your style is fine.
interactWith :: (String -> String) -> FilePath -> FilePath -> IO ()
interactWith f input output = readFile input >>= writeFile output . f

In short:

  • add type signatures to top-level functions
  • don't use head on lists that might be empty
  • use standard library types when possible (Maybe)
  • use standard library functions when possible (map, lines and mapMaybe)
answered Oct 29, 2018 at 13:26
\$\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.