Hacker News new | ask | show | jobs
by sbov 2952 days ago
> “There’s a bug over there because you re-used lastCachedTime. Otherwise, looks good. Ship it!”

The fixed code still has a bug. It assumes successive calls to lastMidnight() within the method will always return the same date. But presumably, if partway through the method it ticks over to midnight, that would not be the case. So some stats would be updated during one call, and the rest would be updated during the next call.

That would violate this requirement:

> They had previously decided all stats should refresh simultaneously;

1 comments

He does point out every good programer knows they should have one if check for all three: " Wrapping Up . Many programmers will get a sense that something was off about the first example. Every programmer I’ve shown this example to knew they should merge the if-statements and factor out the prints. "

Which corresponds to the one if part of the story: " if (lastCachedTime <= lastMidnight() || lastCachedTime <= lastNoon()) { numUsers = countUsers(); numArticles = countArticles(); numWords = countWords(); lastCachedTime = Time.now(); } "

So having the multiple ifs being bad even before a second cache refresh is added is part of the story.