4
\$\begingroup\$

This is my very first complete, non-completely-trivial Haskell program, and I'm extremely unfamiliar with the language. I was hoping that I could get some pointers on how to make it more idiomatic. I'm especially unfamiliar with built-in functions.

I'm specifically concerned with my main function and my scrollHelp guards, as it feels like there should be a better way to do both. Also the naming of the scroll and scrollHelp functions is arbitrary, and they might be able to be combined.

The program itself is very simple, it takes a numerical width and a string as command line parameters and transforms that string into a fake marquee.

Example:

./scroll 4 testing

[    ]
[   t]
[  te]
[ tes]
[test]
[esti]
[stin]
[ting]
[ing ]
[ng  ]
[g   ]
[    ]

scroll.hs

import System.Environment
import System.Exit
import System.IO

slice :: Int -> Int -> [Char] -> [Char]
slice from to xs = take to $ drop from $ xs

slice0 :: Int -> [Char] -> [Char]
slice0 = slice 0

join :: [a] -> [[a]] -> [a]
join a xss = concatMap (++ a) xss

spaces :: Int -> [Char]
spaces a = concat $ take a $ repeat " "

lead = "    ["
end = "]"

format :: [Char] -> [Char]
format xs = lead ++ xs ++ end

scroll :: Int -> [Char] -> [[Char]]
scroll a [] = [format $ spaces a]
scroll a x = scrollHelp a (spaces a ++ x ++ spaces a)

scrollHelp :: Int -> [Char] -> [[Char]]
scrollHelp a xall@(_:xs)
  | length xall >= a = (format $ slice0 a xall) : scrollHelp a xs
  | otherwise      = []
scrollHelp a _ = []

main = do
  args <- getArgs
  case args of
    [widthString, text] | [(width,_)] <- reads widthString ->
      putStrLn $ join "\n" (scroll width text)
    _ -> do
      name <- getProgName
      hPutStrLn stderr $ "usage: " ++ name ++ " <marquee width> <text to marquee>"
      exitFailure
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  • use replicate a ' ' instead of concat $ take a $ repeat " "
  • use pattern matching in main to check that there are exactly two arguments and use it also for read to check that the width is exactly a number. With [(width,_)] you accept also strings that only start with a number, such as "1O" where probably you made a mistake and you wrote it instead of "10"
  • lead and end can be specific to format to avoid exposing them. Actually you can also remove them and just write " [" ++ xs ++ "]"
  • scroll can be simplified and both the helper and the format function can should be only visible to it
  • don't concat the output together before writing it: just write each line in output. UseforM_ for multiple IO operations over a list of values.

So, my rewrite of your code is

import Control.Monad (forM_)
import System.Environment
import System.Exit
import System.IO


scroll :: Int -> [Char] -> [[Char]]
scroll a = reverse . scrollHelp [] . addSpaces
  where
    addSpaces xs = replicate a ' ' ++ xs ++ replicate a ' '
    scrollHelp acc str
      | length str < a = acc
      | otherwise      = let group = take a str
                         in scrollHelp (format group:acc) (tail str)
    format xs = "    [" ++ xs ++ "]"

main = do
  args <- getArgs
  case args of
    (widthString:text:[]) ->
       case reads widthString of
         [(width,"")] -> forM_ (scroll width text) putStrLn 
         _ -> hPutStrLn stderr $ "width must be an integer, got '" ++ widthString ++ "'"
    _ -> do
      name <- getProgName
      hPutStrLn stderr $ "usage: " ++ name ++ " <marquee width> <text to marquee>"
      exitFailure
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.