Dynamic IP-address logging

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  String
extractFromFile "" = ""
extractFromFile s = (fromMaybe [] . listToMaybe . words . last . lines) s

extractIp :: String -> String
extractIp "" = ""
extractIp s = (takeWhile (/=' IO String
myReadFile fp = do
    iplogExists  IO String
getCurlIP url = do
    (curlState, curlString)  String
getLocalIP = show . ipv4 . head . filter (\x -> name x == "eth0")

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.

-- "Current IP Check
--  Current IP Address: 24.20.128.212\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  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

StackExchange Code Review Q#12515, answer score: 3