Is this a good interface for a simple class?

I made a class that caches and downloads a string. I know that Kotlin is all about succinctness and I’m fairly new to Kotlin, so I don’t have the “muscle memory” yet to know if this is how such a class would be instantiated and used.

Class:

class Cacher (
    private val appl: Application,
    private val cacheKey: String,
    private val url: String,
) {
    suspend fun get(): String? {
        [...]
    }
}

Usage:

val s = Cacher(
    requireActivity().application,
    "example_cache",
    "https://example.com"
).get()

API is pretty simple, so it is hard to make it more idiomatic. My two cents are related to OOP in general.

1. It feels like a pretty heavy thing for just downloading a single string. You could consider creating a single cacher object that has a map key->url and then you use it like this:

cacher.get("example_cache")

2. If you want to keep it as it is, then I think this class really performs two unrelated things that could/should be separated: downloading with HTTP; and caching with suspending and synchronization. You could consider creating a more generic function for suspendable caching like this:

fun <T> suspendLazy(init: suspend () -> T): SuspendLazy<T> { TODO() }

interface SuspendLazy<T> {
    suspend fun get(): T
}

Then you can use it directly, ignoring your Cacher class entirely:

val cachedString = suspendLazy { http.get("https://example.com") }

Or you can create cacher on top of it:

fun cacher(appl: Application, cacheKey: String, url: String) = suspendLazy { http.get(url) }

You could also consider not creating SuspendLazy interface, but reusing Deferred. It is a little different as it allows to cancel the execution, so depending on your case it may be good or bad thing.

Also, do you really need Application there? Isn’t Context enough?

1 Like

The caching and the downloading are quite entangled. When the cache is cold, it just downloads the resource and stores it together with the Expires and Date header. When the resource is in the cache, and the Expires hasn’t passed it returns the cached value. If it has expired, it downloads the resource but this time with the Date header in If-Modified-Since. If it gets a 304 Not Modified it returns the cached value, otherwise it caches and returns the new resource.
It would be an interesting exercise to split the responsibilities but it also seems like false abstraction.

You’re right.

Ahh, ok, in that case it makes sense to use a separate object per string and suspendLazy() won’t give you too much.

One last suggestion. You assume the value may change with time and this is so important in your application logic that you even decided to implement this into your Cacher. If you know you may sometimes need to observe the value of cached string as it is being updated, then it would make sense to return StateFlow<String?> instead of just String (null is for unknown). With such flow it is extremely easy to both suspend for a single value and subscribe to observe changes. But I imagine it would be harder to implement.

I am actually using this cacher with a flow. It looks basically like this:

object ServerConnection {

    private val _stateUpdates = MutableSharedFlow<State>()
    val stateUpdates = _stateUpdates.asLiveData()

    init {
        CoroutineScope(Dispatchers.IO).launch {
            // Retry loop
            while (true) {
                try {
                    while(true) {
                        val value = cacher.get()
                        _stateUpdates.emit(value)
                        delay(60_000)
                    }
                }
            }
        }
    }
}

(I omitted some additional processing that happens with the value before it is emitted.)

If I remember correctly, the way I arrived at this code was: I knew I had to run it on the IO threadpool, but then I couldn’t emit to the main thread, so I added a Channel, but with a Channel only one of the receivers got the updated value, so I googled and found the BroadcastChannel, which was deprecated in favor of the MutableSharedFlow. I discovered that is has a very convenient .asLiveData() method, so that’s what I’m using now.

The UI then subscribes to updates with ServerConnection.stateUpdates.observe(viewLifecycleOwner, { ... })