epam / epam/CoroutinesCache

Library feedback & future improvements

Open
#5 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Kotlin
Stars
165
Forks
7
PR merge metrics
No merged PRs in 30d

Description

Hi,

Great library, I had a brief look into the source code, here are some of my thoughts:

**Dependencies**
```
implementation 'com.google.code.gson:gson:2.8.5'
implementation 'com.squareup.moshi:moshi:1.6.0'
implementation 'com.fasterxml.jackson.core:jackson-core:2.9.6'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.9.6'
```
Not everyone is happy by bringing unnecessary dependencies, better to put them as separate libraries. E.g. `com.epam.coroutinecache:coroutines-cache-gson-mapper`

**API**
1. `CoroutinesCache` has a `CoroutineScope` parameter. I think cache operations should be scoped, not enire cache. In most cases, cache is a singleton and we create it once per app lifetime since initialization of this object usually takes some time.
Also, you default to `GlobalScope` which is represented by `Dispatchers.Default`, in your case you should be using `Dispatchers.IO`
2. Would be nice to be able to save some android data structures like `Bitmap`.

**Performance**
1. `LRU` implementation of memory and disk cache would be great.
2. Currently, every operation lock memory and disk cache. You can boost performance a lot if you perform lock on particular key, not entire store.

E.g.
```kotlin
// independent operations, different keys
store.put("key1")
store.get("key2")

// dependent operations, same keys
store.put("key1")
store.get("key1")

// dependent operations, global
store.put("key1")
store. deleteAll()
```

3. You expose coroutine API, internally use [Mutex](https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/) as a locking mechanism.
4. Instead of trying to figure out data type during serialization/deserialization I suggest to just ask user this infromation. E.g. why not to do `@ProviderKey("TestKey", "Bitmap::class")`? With this approach you don't need reflection and can just immediately retrieve correct serializer/deserializer.

**Realiability**
For `DiskCache` it's better to use [AtomicFile](https://developer.android.com/reference/android/support/v4/util/AtomicFile) to have a backup.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.