patternModerate
Periodically comparing the play count of two Last.FM users
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
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
Note that the type annotation on
I personally might make a few more refactorings, but they aren't really necessary:
The type signature on
<- "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 microsecondsThe 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 microsecondsContext
StackExchange Code Review Q#10327, answer score: 10
Revisions (0)
No revisions yet.