HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Periodically comparing the play count of two Last.FM users

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastthecomparingplaytwoperiodicallycountusers

Problem

There is some little program (using a Last.FM API wrapper) which gets the play count of 2 specific users every 5 minutes and appends the difference between these 2 numbers to log file.

Can you review that part of the code?
I think that using the <- operator so many times decreases the readability and simplicity of the code.

main = forever $ do
  diff  getZonedTime
  let logMessage = printf "%s %d\n" timestamp diff :: String
  homeDir  logFile
  appendFile logFilePath logMessage
  threadDelay $ 10^6 * 60 * timeDelay -- in microseconds
    where
      subtractCounts = liftM2 ((-) `on` fromMaybe 0) `on` getPlayCount
      dateFormat = "[%s] %D %H:%M"

Solution

I don't see why you say <- "decreases readability", but you could line them up if you think that helps (this style is typical of Haskell programmers)

main = forever $ do
  diff       getZonedTime
  homeDir    logFile
  appendFile logFilePath logMessage
  threadDelay $ 10^6 * 60 * timeDelay -- in microseconds

subtractCounts = liftM2 ((-) `on` fromMaybe 0) `on` getPlayCount
dateFormat     = "[%s] %D %H:%M"


<- is a visual cue that the thing on the right might to perform an effect before producing a result and binding it to the name on the left.

Note that the type annotation on logMessage is unnecessary. Because it is used as the second argument to appendFile it will be inferred to be String.

I personally might make a few more refactorings, but they aren't really necessary:

logMessage :: String -> Integer -> String
logMessage  = printf "%s %d\n"

(f .: g) x y = f $ g x y

subtractCounts = liftM2 ((-) `on` fromMaybe 0) `on` getPlayCount

main = forever $ do
  homeDir  logFile
      logDiff     = appendFile logFilePath .: logMessage
  forever $ mainLoop logDiff

mainLoop logDiff = do
  diff       getZonedTime
  logDiff timestamp diff
  threadDelay $ 10^6 * 60 * timeDelay -- in microseconds


The type signature on logMessage is probably unnecessary, but imho it's a good idea to give it an explicit signature if you are using the result of printf as a function, rather than a String or IO ().

Code Snippets

main = forever $ do
  diff      <- user1 `subtractCounts` user2
  timestamp <- formatTime defaultTimeLocale dateFormat <$> getZonedTime
  homeDir   <- getHomeDirectory
  let logMessage  = printf "%s %d\n" timestamp diff
      logFilePath = homeDir </> logFile
  appendFile logFilePath logMessage
  threadDelay $ 10^6 * 60 * timeDelay -- in microseconds

subtractCounts = liftM2 ((-) `on` fromMaybe 0) `on` getPlayCount
dateFormat     = "[%s] %D %H:%M"
logMessage :: String -> Integer -> String
logMessage  = printf "%s %d\n"

(f .: g) x y = f $ g x y

subtractCounts = liftM2 ((-) `on` fromMaybe 0) `on` getPlayCount


main = forever $ do
  homeDir <- getHomeDirectory
  let logFilePath = homeDir </> logFile
      logDiff     = appendFile logFilePath .: logMessage
  forever $ mainLoop logDiff

mainLoop logDiff = do
  diff      <- user1 `subtractCounts` user2
  timestamp <- formatTime defaultTimeLocale "[%s] %D %H:%M" <$> getZonedTime
  logDiff timestamp diff
  threadDelay $ 10^6 * 60 * timeDelay -- in microseconds

Context

StackExchange Code Review Q#10327, answer score: 10

Revisions (0)

No revisions yet.