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)
        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

Right from the top I notice you could benefit from some factoring.

prompt :: String -> IO String
prompt str = do
    putStr str
    hFlush stdout

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"

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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