Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#114 Add in memory preference #125

Closed
wants to merge 6 commits into from
Closed

#114 Add in memory preference #125

wants to merge 6 commits into from

Conversation

kirillzh
Copy link
Contributor

@kirillzh kirillzh commented Mar 18, 2019 •
edited
Loading

Adding in-memory implementation of the Preference that can be used for testing purposes.
Closes #114

Here's a primitive example of how and why this can be used. Let's say we have some dummy class that is backed by a boolean flag and boolean Preference:

class Foo(val pref: Preference<Boolean>) {
    private var localFlag: Boolean = false

    val isEnabled: Boolean get() = localFlag && pref.get()

    fun enable() = set(true)
    fun disable() = set(false)

    private fun set(state: Boolean) {
        this.localFlag = state
        pref.set(state)
    }
}

To test Foo#isEnabled property we need some implementation of the Preference which would persist and change its state accordingly:

val pref = TestPreference("key", false)
val foo = Foo(pref)

assertThat(foo.isEnabled).isFalse()
foo.enable()
assertThat(foo.isEnabled).isTrue()
foo.disable()
assertThat(foo.isEnabled).isFalse()

Some remaining tasks:

  • add usage documentation
  • convert back to Java, maybe?

@kirillzh
Copy link
Contributor Author

Once #124 is merged to master by the owners, unnecessary commits should go away from this PR.

@kirillzh kirillzh changed the title 114/add memory preference #114 Add in memory preference Mar 18, 2019
@kirillzh
Copy link
Contributor Author

Any suggestions where TestPreference.kt should be located? :rx-preferences:androidJavadocs didn't like it when I put the file into test package.

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/test/TestPreference.kt Outdated
* pass the preference as a dependency and don't care about its state, simply mock the preference
* using [Preference] interface.
*/
class TestPreference<T>(private val key: String, private val defaultValue: T) : Preference<T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestions for name?

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/test/TestPreference.kt Outdated
* In-memory implementation of [Preference] that can be used for testing purposes. Only use this
* if your code under test depends on the changes in the preferences. Otherwise, if you need to
* pass the preference as a dependency and don't care about its state, simply mock the preference
* using [Preference] interface.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to get any corrections, if any!

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/test/TestPreference.kt Outdated
values.onNext(defaultValue)
}

override fun asObservable(): Observable<T> = values.distinctUntilChanged()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was thinking if I should store values.distinctUntilChanged() into a property and store it here.

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/test/TestPreference.kt Outdated
*
* @see [asObservable].
*/
private val values = BehaviorSubject.createDefault<T>(defaultValue)
Copy link
Contributor Author

@kirillzh kirillzh Mar 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any RxJava edge cases that I need to look out for?

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/RxSharedPreferences.java
@@ -19,6 +20,7 @@
import static com.f2prateek.rx.preferences2.Preconditions.checkNotNull;

/** A factory for reactive {@link Preference} objects. */
@SuppressWarnings("WeakerAccess")
Copy link
Contributor Author

@kirillzh kirillzh Mar 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes warnings about public APIs not having strict enough visibility. Should this be suppressed per method?

kirillzh
kirillzh commented Mar 18, 2019
View reviewed changes
rx-preferences/build.gradle
compile deps.annotations
compile deps.rxjava
implementation deps.annotations
implementation deps.kotlinStdlib
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be public? TestPreference.kt doesn't expose any APIs from stdlib.

@kirillzh
Copy link
Contributor Author

Any suggestions where TestPreference.kt should be located? :rx-preferences:androidJavadocs didn't like it when I put the file into test package.

It's not package name, it's file name (or both)? Any suggestions? Not sure how to tune it in to allow test/TestPreference.kt naming...

@kirillzh
Copy link
Contributor Author

Considering that I created only one new file, adding Kotlin as a new dependency probably wasn't a good idea, can revert to Java. Thoughts?

f2prateek
f2prateek reviewed Apr 3, 2019
View reviewed changes
rx-preferences/src/main/java/com/f2prateek/rx/preferences2/TestPreference.kt
* pass the preference as a dependency and don't care about its state, simply mock the preference
* using [Preference] interface.
*/
class TestPreference<T>(private val key: String, private val defaultValue: T) : Preference<T> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about naming this MemoryPreference?

f2prateek
f2prateek reviewed Apr 3, 2019
View reviewed changes
build.gradle
compileSdkVersion = 28
java8Version = JavaVersion.VERSION_1_8
javaVersion = JavaVersion.VERSION_1_7
kotlinVersion = '1.3.21'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to add a Kotlin dependency just for this.

@kirillzh kirillzh closed this Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f2prateek f2prateek f2prateek left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add MemoryPreference artifact?
2 participants
@kirillzh @f2prateek