russhwolf / multiplatform-settings

A Kotlin Multiplatform library for saving simple key-value data
Apache License 2.0
1.7k stars 67 forks source link

Add commit attribute to AndroidSettings.Factory #74

Closed xsveda closed 2 years ago

xsveda commented 3 years ago

AndroidSettings constructor allows to set a commit flag.

I am missing the same configuration option when using AndroidSettings.Factory.

As the fun create(String?) is defined in the parent Settings.Factory interface, the commit option would need to be set in AndroidSettings.Factory constructor.

If we can agree on this I can to the PR.

xsveda commented 3 years ago

This might be taken broader to cover all specific configuration options for other Settings implementations. Another example would be useFrozenListeners flag of AppleSettings

russhwolf commented 3 years ago

There's a bit of a combinatorial explosion when trying to support every option for both constructors and factories. I haven't been very focused on the Factory implementations since you can fairly easily create your own if you need different options. I've also become less and less convinced over time that having a Factory interface is really that useful compared to just supplying your own () -> Settings or (String) -> Settings lambda as needed.

At some point I want to do some deeper rethinking of how initialization works, and have been thinking about doing a DSL builder style like Ktor and kotlinx.serialization use. See #65.

francismariano commented 3 years ago

I think the commit flag could set also from common module perspective.

For example, when I use the no-arg module and creating the settings with val settings : Settings = Settings() I think interesting config the commit flag in this point.

russhwolf commented 3 years ago

Platform-specific settings should be configured in platform code, not common code. Otherwise you end up with a bunch of extra flags in common code that don't do anything most of the time, and that makes for a confusing API.

The purpose of no-arg is to provide a reasonable default configuration where you don't need to do anything extra. If those default settings don't fit your needs, then you shouldn't be using no-arg.

russhwolf commented 2 years ago

I don't think there's changes that need to happen here so I'm closing this issue.