Add a wrapper to convert Settings to ObservableSettings by manually invoking callbacks

[russhwolf]

It would be pretty straightforward to create a class that manually wires in update listeners in Kotlin for platforms that don't natively have them.

class ObservableSettingsWrapper(val delegate: Settings): ObservableSettings {
    private val listeners = mutableListOf<() -> Unit>()

    fun addIntListener(key: String, callback: (Int) -> Unit): SettingsListener {
        var prev = getIntOrNull(key)
        listeners.add {
            val current = getIntOrNull(key)
            if (prev != current) {
                block()
                prev = current
            }
        }
        // ...
    }

    fun setInt(key: String, value: Int) {
        delegate.setInt(key, value)
        listeners.forEach { it.invoke() }
    }

    // ...
}

The reason I've never added this to the library previously is, the interop story can get confusing here. For example, if you have platform code in JS interacting with localStorage directly, then making changes there would not trigger your listeners. Indeed even if you had multiple Settings instances reading from the same storage, they wouldn't trigger each other's listeners in the implementation above.

As KMP matures, there's a growing number of users for whom the interop story is less important than having an API that does everything they need in common code. So I'm reconsidering adding an API like this, and want to hear what others in the community think about it.

Some details I'm considering

• Like multiplatform-settings-no-arg, this would probably live in its own module. This way, you must explicitly opt into using it and are more likely to be aware of potential pitfalls. (An opt-in annotation might also help with this)
• What's a better name than ObservableSettingsWrapper?
• Does it feel nicer to construct something like ObservableSettingsWrapper(delegate) or to use an extension api like delegate.makeObservable()? (This can help sidestep the class naming problem but the function still needs a name)
• Should this API be smart enough to no-op when used on something that is already ObservableSettings, or should it always behave the same and it's up to the consumer to know when to use it?

[psuzn]

This would certainly be useful. I’ve created a simpler version of this and would be happy to contribute if you decide to include it in the pipeline.

[russhwolf]

I don't want to think too much about this until after getting 1.1 out the door, but would certainly welcome contributions. Is the version you've used available publicly anywhere?

[psuzn]

It is not available publicly yet. I'm working on a simple multiplatform app that will eventually be public.

[psuzn]

I finally published my app and here is the link to my implementation.

Besides this, I also ended up adding some extensions to support stateflow.

I can contribute these back if you think it adds some value to this project.

[russhwolf]

Hi @psuzn,

Sorry it's taken me a while to get back on this. I'd like to add something like this, and would welcome the contribution if you'd like to make a PR with your implementation.

Some notes:

• I like your use of a map of listeners where I've usually used a list. That should be more efficient if there are a lot of them.
• There are several things I'd like to adjust in the Listener class:
  • It shouldn't be open. There's no need for extensibility that I can see.
  • It also shouldn't be inner. You could drop this from the Listener implementation by passing the listeners map to it, similar to MapSettings. It only needs access to that map, and not the rest of the outer class.
  • I'd like to avoid the type parameter in the public interface. The Listener implementation is essentially just a reference to call deactivate() on. Users shouldn't need to supply a type parameter when interacting with it. (I acknowledge that the interface of this class doesn't matter a ton because the parent class is private, but I'd like the flexibility to change that in the future)
• I'm not sure what I want to do for naming yet, but definitely like having a private class with an extension function or factory function to create it

[ChristianKatzmann]

@psuzn Hey there! Are you by any chance about to create the aforementioned PR? Don't want to stress, though.

[psuzn]

Hey, @ChristianKatzmann I started to work on it a while ago, but something came up and I forgot. Will continue with it over the weekends and hopefully will push something soon.

[psuzn]

Since #184 has been merged, let's also close this.

[russhwolf]

Will close once I get it released (which I'm admittedly way late on)

[psuzn]

makes sense, what are other things in the pipeline if there is anything I can help with?