Shouldn't IBlobCache.GetCreatedAt return Observable.Empty or a KeyNotFoundException?

[haacked]

When I call IBlobCache.GetCreatedAt with a key that doesn't exist, I get an observable sequence with one null value. That's counter intuitive. For example, when I call IBlobCache.GetAsync with a key that doesn't exist, I get an observed KeyNotFoundException.

For GetCreatedAt I would have expected one of these behaviors.

This brings up a general question about Rx operators. When you have an operator that returns a single result, when should you return Observable.Return(null) and when should you do Observable.Empty()? I'm finding that the lack of consistency with this leads me to have to write extra defensive boiler-plate code since I'm never sure what any given operator will do.

Thoughts?

[haacked]

My vote is to never return null and let others handle the exception and return null if that's what they want. That way no information is lost.

[anaisbetts]

The history of this method isn't Great - originally GetCreatedAt was solely for debugging purposes or scenarios where the data isn't in motion (i.e. initialization / migration type'y things). Then, the idea was more to be able to check whether a key existed without throwing, but then in the SQLite3 backend there was a bug where we weren't saving Created At dates (#90). This method would probably be seen on back episodes of COPS, it's kind of a mess.

This brings up a general question about Rx operators. When you have an operator that returns a single result, when should you return Observable.Return(null) and when should you do Observable.Empty()

I'd say it should always return Observable.Return(null), returning Empty should only be for observables that can return more than one item. Almost all of the IObservables in Akavache have "Task<T> Nature™", not "Event Stream Nature™", except for GetAndFetchLatest

[haacked]

But doesn't a Task that can't return a value throw? My concern is that as a subscriber, I may need to understand why it couldn't return a value. Null doesn't tell me that.

As an aside, in some code I'm writing I ended up writing a IBlobCache.GetValueAndCacheDate method. Might be nice to encapsulate that into Akavache if you're interested. That way you can either just get the cached value, or get a CacheEntry that gives you the value, cache date, and expiration. I'd be interested in such a refactoring if you think it's feasible and worthy.

[anaisbetts]

My concern is that as a subscriber, I may need to understand why it couldn't return a value. Null doesn't tell me that.

In this case I agree we should change it to throwing. GetCreatedAt definitely doesn't represent Rx Best Practices™. It might be better to burn down the entire concept and think about use-cases again

[haacked]

I'd say it should always return Observable.Return(null), returning Empty should only be for observables that can return more than one item.

The more I think about this, the more I disagree in the general case. I think this is analogous to collections. A public member or method that has a collection interface as a return type should never return null. We know when they do, that's the work of Satan. :stuck_out_tongue:

In this case I agree we should change it to throwing. GetCreatedAt definitely doesn't represent Rx Best Practices™. It might be better to burn down the entire concept and think about use-cases again

Consider me nerd sniped. I'll be getting on a plane soon, but when I get back, I'll try and look at this. I think my CacheEntry idea makes sense. Most of the use cases are around folks who want to build their own cache strategies operators involving IBlobCache.