From 4a2b859cad2890cef9572f96b7d1644675b68d90 Mon Sep 17 00:00:00 2001 From: Daan Vanden Bosch Date: Tue, 1 Nov 2022 21:55:48 +0100 Subject: [PATCH] Added some observe extension methods, added a mutation test and removed old commented-out code. --- .../kotlin/world/phantasmal/cell/Cells.kt | 49 ++++++-- .../kotlin/world/phantasmal/cell/CellTests.kt | 40 +++++-- .../world/phantasmal/cell/MutableCellTests.kt | 112 ------------------ 3 files changed, 74 insertions(+), 127 deletions(-) diff --git a/cell/src/commonMain/kotlin/world/phantasmal/cell/Cells.kt b/cell/src/commonMain/kotlin/world/phantasmal/cell/Cells.kt index 1578f775..7a495856 100644 --- a/cell/src/commonMain/kotlin/world/phantasmal/cell/Cells.kt +++ b/cell/src/commonMain/kotlin/world/phantasmal/cell/Cells.kt @@ -48,10 +48,46 @@ fun mutableCell(getter: () -> T, setter: (T) -> Unit): MutableCell = fun Cell.observe(observer: (T) -> Unit): Disposable = observeChange { observer(it.value) } +fun observe( + c1: Cell, + c2: Cell, + observer: (T1, T2) -> Unit, +): Disposable = + CallbackObserver(c1, c2) { observer(c1.value, c2.value) } + +fun observe( + c1: Cell, + c2: Cell, + c3: Cell, + observer: (T1, T2, T3) -> Unit, +): Disposable = + CallbackObserver(c1, c2, c3) { observer(c1.value, c2.value, c3.value) } + +fun observe( + c1: Cell, + c2: Cell, + c3: Cell, + c4: Cell, + observer: (T1, T2, T3, T4) -> Unit, +): Disposable = + CallbackObserver(c1, c2, c3, c4) { observer(c1.value, c2.value, c3.value, c4.value) } + +fun observe( + c1: Cell, + c2: Cell, + c3: Cell, + c4: Cell, + c5: Cell, + observer: (T1, T2, T3, T4, T5) -> Unit, +): Disposable = + CallbackObserver(c1, c2, c3, c4, c5) { + observer(c1.value, c2.value, c3.value, c4.value, c5.value) + } + fun Cell.observeNow( observer: (T) -> Unit, ): Disposable { - val disposable = observeChange { observer(it.value) } + val disposable = observe(observer) // Call observer after observeChange to avoid double recomputation in most cells. observer(value) return disposable @@ -62,7 +98,7 @@ fun observeNow( c2: Cell, observer: (T1, T2) -> Unit, ): Disposable { - val disposable = CallbackObserver(c1, c2) { observer(c1.value, c2.value) } + val disposable = observe(c1, c2, observer) // Call observer after observeChange to avoid double recomputation in most cells. observer(c1.value, c2.value) return disposable @@ -74,7 +110,7 @@ fun observeNow( c3: Cell, observer: (T1, T2, T3) -> Unit, ): Disposable { - val disposable = CallbackObserver(c1, c2, c3) { observer(c1.value, c2.value, c3.value) } + val disposable = observe(c1, c2, c3, observer) // Call observer after observeChange to avoid double recomputation in most cells. observer(c1.value, c2.value, c3.value) return disposable @@ -87,8 +123,7 @@ fun observeNow( c4: Cell, observer: (T1, T2, T3, T4) -> Unit, ): Disposable { - val disposable = - CallbackObserver(c1, c2, c3, c4) { observer(c1.value, c2.value, c3.value, c4.value) } + val disposable = observe(c1, c2, c3, c4, observer) // Call observer after observeChange to avoid double recomputation in most cells. observer(c1.value, c2.value, c3.value, c4.value) return disposable @@ -102,9 +137,7 @@ fun observeNow( c5: Cell, observer: (T1, T2, T3, T4, T5) -> Unit, ): Disposable { - val disposable = CallbackObserver(c1, c2, c3, c4, c5) { - observer(c1.value, c2.value, c3.value, c4.value, c5.value) - } + val disposable = observe(c1, c2, c3, c4, c5, observer) // Call observer after observeChange to avoid double recomputation in most cells. observer(c1.value, c2.value, c3.value, c4.value, c5.value) return disposable diff --git a/cell/src/commonTest/kotlin/world/phantasmal/cell/CellTests.kt b/cell/src/commonTest/kotlin/world/phantasmal/cell/CellTests.kt index e5d44e20..c4086a97 100644 --- a/cell/src/commonTest/kotlin/world/phantasmal/cell/CellTests.kt +++ b/cell/src/commonTest/kotlin/world/phantasmal/cell/CellTests.kt @@ -2,12 +2,7 @@ package world.phantasmal.cell import world.phantasmal.cell.test.CellTestSuite import world.phantasmal.core.disposable.use -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertNotEquals -import kotlin.test.assertNotNull -import kotlin.test.assertNull -import kotlin.test.assertTrue +import kotlin.test.* /** * Test suite for all [Cell] implementations. There is a subclass of this suite for every [Cell] @@ -243,7 +238,7 @@ interface CellTests : CellTestSuite { } ) - // Repeat 3 times to check if temporary state is always reset. + // Repeat 3 times to check that temporary state is always reset. repeat(3) { changes = 0 @@ -261,6 +256,37 @@ interface CellTests : CellTestSuite { } } + /** + * When two dependencies of an observer are changed in a single mutation, the observer is called + * just once. + */ + @Test + fun changing_two_dependencies_during_a_mutation_results_in_one_callback_call() = test { + val p1 = createProvider() + val p2 = createProvider() + var callbackCalled = 0 + + disposer.add( + observe(p1.cell, p2.cell) { _, _ -> callbackCalled++ } + ) + + // Repeat 3 times to check that temporary state is always reset. + repeat(3) { + callbackCalled = 0 + + mutate { + p1.emit() + p2.emit() + + // Change should be deferred until this mutation finishes. + assertEquals(0, callbackCalled) + } + + // All changes should result in a single callback call. + assertEquals(1, callbackCalled) + } + } + @Test fun value_can_be_accessed_during_a_mutation() = test { val p = createProvider() diff --git a/cell/src/commonTest/kotlin/world/phantasmal/cell/MutableCellTests.kt b/cell/src/commonTest/kotlin/world/phantasmal/cell/MutableCellTests.kt index fc493f58..1bbf4c28 100644 --- a/cell/src/commonTest/kotlin/world/phantasmal/cell/MutableCellTests.kt +++ b/cell/src/commonTest/kotlin/world/phantasmal/cell/MutableCellTests.kt @@ -25,118 +25,6 @@ interface MutableCellTests : CellTests { assertEquals(newValue, observedValue) } - // TODO: Figure out change set bug and enable change sets again. - /** - * Modifying mutable cells in a change set doesn't result in calls to - * [Dependent.dependencyChanged] of their dependents until the change set is completed. - */ -// @Test -// fun cell_changes_in_change_set_dont_immediately_produce_dependencyChanged_calls() = test { -// val dependencies = (1..5).map { createProvider() } -// -// var dependencyMightChangeCount = 0 -// var dependencyChangedCount = 0 -// -// val dependent = object : Dependent { -// override fun dependencyMightChange() { -// dependencyMightChangeCount++ -// } -// -// override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { -// dependencyChangedCount++ -// } -// } -// -// for (dependency in dependencies) { -// dependency.observable.addDependent(dependent) -// } -// -// change { -// for (dependency in dependencies) { -// dependency.observable.value = dependency.createValue() -// } -// -// // Calls to dependencyMightChange happen immediately. -// assertEquals(dependencies.size, dependencyMightChangeCount) -// // Calls to dependencyChanged happen later. -// assertEquals(0, dependencyChangedCount) -// } -// -// assertEquals(dependencies.size, dependencyMightChangeCount) -// assertEquals(dependencies.size, dependencyChangedCount) -// } - - // TODO: Figure out change set bug and enable change sets again. - /** - * Modifying a mutable cell multiple times in one change set results in a single call to - * [Dependent.dependencyInvalidated] and [Dependent.dependencyChanged]. - */ -// @Test -// fun multiple_changes_to_one_cell_in_change_set() = test { -// val dependency = createProvider() -// -// var dependencyMightChangeCount = 0 -// var dependencyChangedCount = 0 -// -// val dependent = object : Dependent { -// override fun dependencyMightChange() { -// dependencyMightChangeCount++ -// } -// -// override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { -// dependencyChangedCount++ -// } -// } -// -// dependency.observable.addDependent(dependent) -// -// // Change the dependency multiple times in a transaction. -// change { -// repeat(5) { -// dependency.observable.value = dependency.createValue() -// } -// -// // Calls to dependencyMightChange happen immediately. -// assertEquals(1, dependencyMightChangeCount) -// // Calls to dependencyChanged happen later. -// assertEquals(0, dependencyChangedCount) -// } -// -// assertEquals(1, dependencyMightChangeCount) -// assertEquals(1, dependencyChangedCount) -// } - - // TODO: Figure out change set bug and enable change sets again. - /** - * Modifying two mutable cells in a change set results in a single recomputation of their - * dependent. - */ -// @Test -// fun modifying_two_cells_together_results_in_one_recomputation() = test { -// val dependency1 = createProvider() -// val dependency2 = createProvider() -// -// var computeCount = 0 -// -// val dependent = DependentCell(dependency1.observable, dependency2.observable) { -// computeCount++ -// Unit -// } -// -// // Observe dependent to ensure it gets recomputed when its dependencies change. -// disposer.add(dependent.observe {}) -// -// // DependentCell's compute function is called once when we start observing. -// assertEquals(1, computeCount) -// -// change { -// dependency1.observable.value = dependency1.createValue() -// dependency2.observable.value = dependency2.createValue() -// } -// -// assertEquals(2, computeCount) -// } - interface Provider : CellTests.Provider { override val cell: MutableCell