feat: added flow wrapper for cache#15
Conversation
b0bbfef to
ffac854
Compare
krogrammer
left a comment
There was a problem hiding this comment.
Copied over my comments from the original PR
| public class CacheFlowWrapper<T>( | ||
| private val cache: SnapshotPersistentCache<T>, | ||
| private val scope: CoroutineScope, | ||
| ) { |
There was a problem hiding this comment.
The API seems like it could be improved by making this class implement MutableStateFlow directly. That would allow for an asFlow() extension on SnapshotPersistentCache:
fileCache.asFlow(scope).collect { it }
public fun <T> SnapshotPersistentCache<T>.asFlow(scope: CoroutineScope): MutableStateFlow<T> {
return CacheFlowWrapper<T>(this, scope)
}And all of the MutableStateFlow members could be implemented by delegating to the internal _cacheValueState
There was a problem hiding this comment.
Okay I gave that a shot, and the only issue I can see with it is that the original implementation has logic around waiting for the first value to be read before updating the flow with anything else. Emit is suspending, and can work the same way, but tryEmit and needing to override value as a var with a public setter that's not suspending is where there is an issue.
For tryEmit we could just return false until initialization is complete, but It's trickier for just assigning a new value to the value var. it could throw an error I suppose, but that feels like it's introducing unnecessary risk.
alternatively it could just implement SharedFlow, but that would remove the ability to just read the value directly if needed. Or maybe that could be a separate wrapper for a different use case.
ffac854 to
d6559c1
Compare
feat: adding a wrapper to allow clients to receive all updates to a given cache via a StateFlow