Dynamic IP-address logging

Question

Inspired by Don Stewart's article on Scripting with Haskell I tried a rewrite of a shell-script I did the other day. I was really astonished how easily it worked but as a whole it is about double the size of my shell script and some things feel a bit "clumsy".

• myReadTile and getCurlIP have IO-return type, which seems a bit "unhaskellish"
• extractIp and extractFromFile seem a bit unelegant especially the maybe parts and the trivial case "" to prevent last from throwing an error.

To run my code the packages curl network-info had to be installed - the rest was already there (Debian haskell-platform 2012.2.0.0, ghc-7.4.1)

What my code does:
I want to log the IP-address my provider gives me - and when it changes; as I have a dynamic IP. With this information I can ssh from other devices to my home PC, and I know how long the periods are in which my IP doesn't change.

Code

import System.IO         (readFile, appendFile)
import System.Exit       (exitSuccess)
import System.Directory  (doesFileExist, getHomeDirectory)
import Data.Maybe        (listToMaybe, fromMaybe)
import Data.Time         (getZonedTime)
import Network.BSD       (getHostName)
import Network.Curl      (curlGetString, URLString, CurlOption(CurlTimeout))
import Network.Curl.Code (CurlCode(CurlOK))
import Network.Info      (getNetworkInterfaces, NetworkInterface, name, ipv4)

localFilePath :: FilePath
localFilePath = "/Dropbox/iplog.log"

dyndns :: URLString
dyndns = "http://checkip.dyndns.org"

main :: IO ()
main = do
    home <- getHomeDirectory
    let iplogFilePath = home ++ localFilePath
    iplogS <- myReadFile iplogFilePath
    curlS <- getCurlIP dyndns
    let oldIP     = extractFromFile iplogS
        currentIP = extractIp curlS
    if oldIP /= currentIP
      then do
        date <- fmap show getZonedTime
        host <- getHostName
        localIP <- fmap getLocalIP getNetworkInterfaces
        let content = unwords [currentIP, date, host, localIP, "\n"]
        appendFile iplogFilePath content
      else
        exitSuccess

extractFromFile :: String -> String
extractFromFile "" = ""
extractFromFile s = (fromMaybe [] . listToMaybe . words . last . lines) s

extractIp :: String -> String
extractIp "" = ""
extractIp s = (takeWhile (/='<') . last . words) s

myReadFile :: FilePath -> IO String
myReadFile fp = do
    iplogExists <- doesFileExist fp
    if iplogExists
      then
          readFile fp
      else do
          print "File does not exist - and will be created"
          print ("at: ~" ++ localFilePath)
          return ""

getCurlIP :: URLString -> IO String
getCurlIP url = do
    (curlState, curlString) <- curlGetString url [CurlTimeout 60]
    if curlState == CurlOK
      then
          return curlString
      else do
          print "No external IP found"
          return "0.0.0.0"

getLocalIP :: [NetworkInterface] -> String
getLocalIP = show . ipv4 . head . filter (\x -> name x == "eth0")

Answer 1

You have produced rather nice code, so my review is restricted to just the peripherals.

import System.IO         (readFile, appendFile)
import System.Exit       (exitSuccess)
import System.Directory  (doesFileExist, getHomeDirectory)
import Data.Time         (getZonedTime)
import Data.Char         (isDigit, isSpace)
import Network.BSD       (getHostName)
import Network.Curl      (curlGetString, URLString, CurlOption(CurlTimeout))
import Network.Curl.Code (CurlCode(CurlOK))
import Network.Info      (getNetworkInterfaces, NetworkInterface, name, ipv4)
import Control.Applicative (<$>, <*>)

localFilePath :: FilePath
localFilePath = "/myhaskell/iplog.log"

dyndns :: URLString
dyndns = "http://checkip.dyndns.org"

myEth = "eth0"

It is nicer to separate out self contained functions even if they are of type IO a (You already have some, but take out as much as you can.)

getLogPath :: IO String
getLogPath = flip (++) localFilePath <$> getHomeDirectory 

The warning that there are no external ips should really be elsewhere.

getCurlIP :: URLString -> IO String
getCurlIP url = extract <$> curlGetString url [CurlTimeout 60]
  where extract (CurlOK, str) = str
        extract _ = "0.0.0.0"

getLocalIP :: [NetworkInterface] -> String
getLocalIP = show . ipv4 . head . filter (flip (==) myEth . name)

Your ip extraction can be simplified further.

firstWord :: String -> String
firstWord = takeWhile (not . isSpace)

getFileIP = firstWord

I modified your implementation a bit to make it clear what is happening. We take letters while they are not numbers, and drop the suffix that start with '<' If your parsing goes beyond this, then go for parsec.

-- "<html><head><title>Current IP Check</title></head>
--  <body>Current IP Address: 24.20.128.212</body></html>\r\n"
extractIp :: String -> String
extractIp = takeWhile (/= '<') . dropWhile (not . isDigit)

Readfile shouldn't really say that a file if it does not exist will be created. It should just read the given file and return the value. The check and warning should be done elsewhere.

catFile :: FilePath -> IO String
catFile fp = doesFileExist fp >>= checkExist
  where checkExist True = readFile fp
        checkExist False = return []

checkS s [] = do 
  print ("File does not exist - and will be created\n" ++ "at: ~" ++ s)
  return []
checkS s str = return str

Using auxiliary definitions can make your code read much better. Prefer sequence to do notation when values are not used in intermediate computations. Try to move out of the imperative mindset when using do notation. (the do notation makes it easy to write 'c' in haskell :) ), using auxiliary functions can help you there. (I used case instead of if/then because that is what I prefer, there is no particular reason for that except that it makes it easy to use destructuring easier if I need to.)

main :: IO ()
main = do
    currentIP <- getCurrentIP
    iplogFilePath <- getLogPath
    oldIP <- getSavedIP iplogFilePath
    checkS iplogFilePath oldIP
    case oldIP /= currentIP of
      True -> getContent currentIP >>= appendFile iplogFilePath
      _ -> exitSuccess 
  where getContent ip = unwords <$> sequence [return ip, show <$> getZonedTime, getHostName,
                                  getLocalIP <$> getNetworkInterfaces]
        getSavedIP path = getFileIP <$> catFile path
        getCurrentIP = extractIp <$> getCurlIP dyndns