Skip to content

Instantly share code, notes, and snippets.

@Lazersmoke
Created August 30, 2017 00:59
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Lazersmoke/4b7d034c6c652196faabcf6752484843 to your computer and use it in GitHub Desktop.
Save Lazersmoke/4b7d034c6c652196faabcf6752484843 to your computer and use it in GitHub Desktop.
Code Review of tchoutri's feed-extract

Code Review of tchoutri's feed-extract

Original Code

Error Log

IO String vs IO ()

The vast majority of the errors are because your functions (particularly help and version) are IO String when they should really be IO (). All of the alternatives in a case must have the same type, all the expressions in a do block must have the same base monad, and the last expression in a do block must have the same type as the do block overall. Let's look at your main function:

main :: IO ()
main = do
    args <- getArgs
    case args of
        []              -> getStdInstead
        ("-h":_)        -> help
        ("--help":_)    -> help
        ("-v":_)        -> version
        ("--version":_) -> version
        (path:_) -> do
            doesIt <- doesFileExist path
            if doesIt then
                processArgs args
                putStrLn "[+] Feeds sucessfully exported to ~/feedreader-export.opml"
            else
              putStrLn "[!] Nah, you really need to give a *real* path :/"

You can see that this is a do block in IO, so everything needs to be in IO. All of your possible paths follow this, so you are OK on that front. Next, all case alternatives must be the same type, and the last expression in a do block must be the same type as the do block overall. Here, we run into trouble. The case is the last expression, so all its alternatives must match the type of the do block overall, which is main :: IO (). Immediately, we can see that this does not hold, since version, help, and getStdInstead are all IO String. For version, help, and getStdInstead, this is trivial fix, since they should have been IO () all along, because they are just do a bunch of IO actions and then don't return anything interesting. Just change each of their types to IO (), and all the errors that look like

• Couldn't match type ‘[Char]’ with ‘()’
  Expected type: IO ()
    Actual type: IO String
• In the expression: getStdInstead
  In a case alternative: [] -> getStdInstead

will be resolved.

The final case alternative

The errors that look like

• Couldn't match expected type ‘(String -> IO ())
                                -> [Char] -> IO ()’
              with actual type ‘IO ()’
• The function ‘processArgs’ is applied to three arguments,
  but its type ‘[String] -> IO ()’ has only one
  In the expression:
    processArgs
      args
      putStrLn
      "[+] Feeds sucessfully exported to ~/feedreader-export.opml"

are a little more intimidating, but we can still pick apart what they mean. The crazy type (String -> IO ()) -> [Char] -> IO () is a good hint that we've done something unintentional, since that doesn't look like a reasonable type for any function we've written. The next part, which says The function ‘processArgs’ is applied to three arguments, but its type ‘[String] -> IO ()’ has only one tells us something interesting: we've oversaturated our processArgs function by two arguments. But if we look at the code where the line number indicates,

processArgs args

it looks like we've only applied it to one argument.

The trick is in the last segment of the error. Notice how it formats the expression:

  In the expression:
    processArgs
      args
      putStrLn
      "[+] Feeds sucessfully exported to ~/feedreader-export.opml"

It looks like it has putStrLn and the string on the same level as args. This means it has interpreted out function call as processArgs args putStrLn "[+] Feeds sucessfully exported to ~/feedreader-export.opml". But wait! Those last two arguments are on a seperate line! When you put something on a new line in a do block, it counts as a new statement! Therein lies the problem: you've forgotten a do. Inserting it on line 16 yields the expected result. The same error occurs on line 32, and you may wish to put dos on lines 19 and 35 even though they aren't strictly needed.

The remaining error

The last error is:

/home/tchoutri/dev/Projects/feed-extract/src/Main.hs:36:25: error:
• Couldn't match type ‘Char’ with ‘[Char]’
  Expected type: [String]
    Actual type: String
• In the first argument of ‘processArgs’, namely ‘args’
  In the expression: processArgs args
  In a case alternative: args -> processArgs args

It is telling us that it expected args to be a [String], but it was actually a String on line 36. This corresponds to the fact that you matched on args instead of (path:_) like you did on line 14. You could change the pattern on line 35 to match line 14 to solve this. You may also wish to check that the file exists when receiving the file name through stdin by copying line 15 to immediately after line 35.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment