https://kotlinlang.org
Join Slack
Any better name than KacheableImpl here: <https://github.com/dave08/kacheable>
# naming
d
Any better name than KacheableImpl here: https://github.com/dave08/kacheable
j
Kacheable
➕ 1
d
j
companion object invoke approach
top level factory function approach
Kacheable is the name 😛
d
top level factory function approach
Doesn't that have to be imported every time? (I already have that problem when using the cache itself...)
j
You already have to import 
KacheableImpl
it changes one import to another one
d
Yeah but the IDE isn't too smart about top levels with the same name sometimes... I was considering the companion object invoke, but either way, should I rename KacheableImpl or just be happy it's hidden by the factory method?
j
Well, I think with the companion object invoke approach you need one import less
Copy code
import foo.Kacheable

val cache: Kacheable = Kacheable(...)
vs
Copy code
import foo.Kacheable
import foo.KacheableImpl

val cache: Kacheable = KacheableImpl(...)
KacheableImpl
looks 
internal
. How you implement it in invoke or whatever is more irrelevant, I would not expose the impl
d
In certain cases, I have to manually import things, and the IDE just doesn't know to suggest things.. Yeah maybe the companion object approach is better here...
j
I don't think you would have problems, there are a lot of samples about this in the coroutines APIs for example, 
MutableStateFlow
is a top-level function for example.
d
In the case of invoke I HAVE TO import it: 
import com.github.dave08.kacheable.invoke
since I have invoke on the interface with a parameter that receives the type and a top-level inline fun to use reified to avoid passing that type...
j
that is weird
d
Maybe in this case it's better though...
j
let me open the IDE
image.png,image.png
Even indicating the type it is not necessary the "second import", but it is smart enough to indicate you are using a factory function with the same name so you can remove the type
d
But with companion objects, you're right. I could maybe try that, although I'm still not sure how to use that to instantiate the Noop implementation...
Maybe invoke with params for the real implementation, and a noop() factory function in the companion object for the noop implementation?
j
Move it into the companion, all extension functions need to be imported except if they belong to 
kotlin
package.
I haven't checked the Noop implementation tbh
if you share more details here, maybe I can help
d
The IDE usually suggests them, but not in this case 🙃
Copy code
object NoopKacheable : Kacheable {
    override suspend fun <R> invalidate(vararg keys: Pair<String, List<Any>>, block: suspend () -> R): R =
        block()

    override suspend fun <R> invoke(
        name: String,
        type: KSerializer<R>,
        vararg params: Any,
        saveResultIf: (R) -> Boolean,
        block: suspend () -> R
    ): R = block()
}
Maybe like this:
Copy code
interface Kacheable {
    suspend fun <R> invalidate(vararg keys: Pair<String, List<Any>>, block: suspend () -> R): R

    suspend fun <R> invoke(
        name: String,
        type: KSerializer<R>,
        vararg params: Any,
        saveResultIf: (R) -> Boolean = { true },
        block: suspend () -> R
    ): R

    companion object {
        operator fun invoke(
            store: KacheableStore,
            configs: Map<String, CacheConfig> = emptyMap(),
            getNameStrategy: GetNameStrategy = GetNameStrategy { name, params ->
                if (params.isEmpty())
                    name
                else
                    "$name:${params.joinToString(",")}"
            },
            jsonParser: Json = Json
        ): Kacheable = KacheableImpl(store, configs, getNameStrategy, jsonParser)

        fun noop(): Kacheable = NoopKacheable
    }
}
m
Could you just do top level function? No companion?
Copy code
interface Kacheable {
  //...
}

fun Kacheable(store:KacheableStore, ...): Kacheable = KacheableImpl()
d
Are there any advantages over the companion object?
m
I like it better, plays nicer in KMP as well with expect/actual but overall I'd say they are comparable
➕ 1
d
Btw, I guess noop should really be noOp?
j
I would create a top level function for NoOp
KacheableNoOp
d
I already have an object 
NoopKacheable
... I could change it's name... I guess the companion object DOES have an advantage that if I start typing Kacheable and then a dot, I get to see both possibilities. I'd suppose that's why you reversed the name to KacheableNoOp -- for code completion?
j
Still different, one needs 
()
, the other not. Maybe for consistence it is better to use top level or invoke in both cases, even if it points to the same internal NoOp object
👍🏼 1
d
Thanks a lot for your help guys! I'll go for top level.
👍 2
c
In my library, Pedestal Cache, I did the same as what multiple people have already commented here: only a single 
Cache
interface is exposed as part of the API, all implementations are private or internal and simply expose a top-level function to instantiate them (example). This allows me to entirely change or even remove implementations as I want. The only thing I have to keep the same between versions are the top-level functions.
👍🏼 1
d
Now I have another little question... should I make the cache's name generic to encourage using something like an enum for it, or maybe pass the cache configuration in 
cache(configForFooCache, param1) { // computatation }
or maybe just stick to strings (hopefully one day getting to generate a function for each annotated cached function with typed params using ksp...)
There's the invalidate function that does need the same cache name... so it can be reused in other places for the same cache...
This is not really #naming though... I just realized. Maybe I should ask in #codereview...
4 Views
Open in Slack
PreviousNext
Powered by

kotlinlang

A modern programming language that makes developers happier.

Powered by