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
1 Answer 1
- use
replicate a ' '
instead ofconcat $ 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
andend
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 theformat
function can should be only visible to it- don't concat the output together before writing it: just write each line in output. Use
forM_
for multipleIO
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