Hacker News Reader

Comment by adamtulinius | Hacker News Reader

Your code should be written this way instead:

    if !cache.contains(key) {
      v = db.query("..")
      expiry = calcexpiry(v)
      cache.put(k, v, expiry)
    }
    
    return cache.get(k)

Only one return, and everything is still in the logical order.

View on HN

I don't think there is anything virtuous about a single return in languages that have catch, finally, and garbage collection.

I think it’s a judgement call: I like clearly indicating when there’s always a single output path but wouldn’t use the single return style in that cache example if it involved an extra lookup operation for a value which is already known.

Yes - the cache example is bad; my point is the general idea of avoiding branches that merge back into the main function.

In your example above, the branch does merge back, and clearly that's fine for a small function - and common pattern - like this. But the general idea would be to avoid it if possible, because that removes unnecessary complexity.

I have used this pattern too. I like it.

One small problem is that depending on how your threading and cache works it might be possible for the cache to be cleared after the contains call but before the get.

Or for the put operation to finish first every time on development machines and only sometimes in production, thus creating a very tricky bug.

IMO, both examples are bad because they search for the key twice.

That’s inefficient and, worse, introduces a concurrency bug. The cached value may expire between the check using ‘contains’ and the call to ‘get’.

A good API would have something akin to C#’s TryGet (https://msdn.microsoft.com/en-us/library/bb347013(v=vs.110)....).

imho, a get() from cache when you already have the value seems like an unnecessary waste of cycles and network traffic.

plus this arrangement would seem vulnerable to a pathological case where you found the V from the db, but have a full/failed/partitioned cache and end up faulting or otherwise not returning the V.

Reply on news.ycombinator.com