5 ms·
Your code should be written this way instead: if !cache.contains(key) { v = db.query("..") expiry = calcexpiry(v) cache.put(k, v, expiry)
by adamtulinius 8y ago
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.
- jakewins 8y agoYes - 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.
- recursive 8y agoI don't think there is anything virtuous about a single return in languages that have catch, finally, and garbage collection.
- acdha 8y agoI 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.
- andy_boot 8y agoI 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.
- jrumbut 8y agoOr for the put operation to finish first every time on development machines and only sometimes in production, thus creating a very tricky bug.
- abiox 8y agoimho, 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.
- Someone 8y agoIMO, 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).aspx https://msdn.microsoft.com/en-us/library/bb347013(v=vs.110)....).