As a Haskell beginner I'm currently playing around the IO monad and prepared simple program which gets from the user two information:
- URL (http)
- Output file path
Then it performs GET request to the provided URL and stores the result in the selected file. HTTP status code of the response is displayed on the standard output
Could you please suggest how such code could be improved? Maybe some places when point-free style can be used? I also feel that the part with confirmation of file overwriting is not very elegant.
import System.IO
import Network.HTTP
import System.Directory
getUrl :: IO String
getUrl = putStr "URL: " >> hFlush stdout >> getLine
confirmOverideOrGetNewPath :: String -> IO String
confirmOverideOrGetNewPath filepath = do
putStr $ "File " ++ filepath ++ " exists. Override? [y/n] "
hFlush stdout
decision <- getLine
let shouldOverride = (if decision == "y" then True else False)
case shouldOverride of True -> return filepath
False -> putStrLn "\nProvide other path" >> getOutputFilepath
getOutputFilepath :: IO String
getOutputFilepath = do
putStr "Output file path: "
hFlush stdout
filepath <- getLine
file_exists <- doesFileExist filepath
case file_exists of True -> confirmOverideOrGetNewPath filepath
False -> return filepath
performGetRequest :: String -> IO (String, Int)
performGetRequest url = do
rsp <- Network.HTTP.simpleHTTP (getRequest url)
content <- getResponseBody rsp
responseCode <- getResponseCode rsp
return (content, responseCodeToInt responseCode)
where
responseCodeToInt :: ResponseCode -> Int
responseCodeToInt code = let (x,y,z) = code in read (mconcat $ show <$> [x,y,z]) :: Int
saveContentToFile :: FilePath -> String -> IO ()
saveContentToFile = writeFile
printHttpStatusCode :: Int -> IO ()
printHttpStatusCode statusCode = putStrLn ("\nStatus code: " ++ show statusCode)
>> putStrLn "Content saved into file"
main :: IO ()
main = do
url <- getUrl
filepath <- getOutputFilepath
(content, statusCode) <- performGetRequest url
saveContentToFile filepath content
printHttpStatusCode statusCode
1 Answer 1
Right from the top I notice you could benefit from some factoring.
prompt :: String -> IO String
prompt str = do
putStr str
hFlush stdout
getLine
Then getURL is prompt "URL: " (and maybe doesn't even need to exist separately!) and confirmOverideOrGetNewPath and getOutputFilepath both lose two lines at the top. You'll notice that I also wrote this function with do-notation, it's a stylistic preference but I just don't like to run monads into one line like that, not sure if I have a great reason why. I guess it just feels right to make imperative-y monadic code look like an imperative language would.
The diversion through shouldOverride in confirmOverideOrGetNewPath is unnecessary, just put your expression in the case-statement.
case decision == "y" of
True -> ...
But then I'd also skip the check for equality entirely and just pattern match on the returned string.
case decision of
"y" -> return filepath
"n" -> getOutputFilepath
_ -> do
putStrLn "Unrecognized input"
confirmOverrideOrGetNewPath filepath
I changed the logic to make the behavior more elucidating. If a user actually enters "n", it's not really necessary to tell them to provide a new path since getOutputFilepath will prompt them for a new one immediately. The user presumably expects that, given their input. If they type something other than "y" or "n", instead of requiring them to input a new path, just check again if they're cool with the override.
In getOutputFilepath all I'd note is that file_exists should be camel cased for consistency.
Over the rest of your program, the only thing I'd switch up would be the whole logic around response codes. In performGetRequest I'd just change the return type to be ResponseCode instead of Int, and push your display logic into printHttpStatusCode (which should probably be printResponseCode). Since ResponseCode is just a type alias for tuples, you can also pattern match in the function call (i.e. in your original version responseCodeToInt (x, y, z) = ... instead of the let). And then it also probably makes more sense to have a pure showResponseCode and leave printing up to the caller.
showResponseCode :: ResponseCode -> String
showResponseCode (i, j, k) = concatMap show [i, j, k]
Then I'd just carry the changes through to main and reorder things for clarity and truth in logic.
main = do
...
(content, statusCode) <- performGetRequest url
putStrLn $ "Response status: " ++ showResponseCode statusCode
writeFile filepath content
putStrLn "Content saved"