Fix #101: use applyOrElse instead of isDefinedAt

OlivierBlanvillain · Apr 17, 2018 · 0e55a54 · 0e55a54
1 parent 606bb8b
commit 0e55a54
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 6 deletions.
diff --git a/monadic-rx/shared/src/main/scala/mhtml/rx.scala b/monadic-rx/shared/src/main/scala/mhtml/rx.scala
@@ -241,12 +241,22 @@ object Rx {
     case Collect(self, f, fallback) =>
       var first = true
       run(self) { a =>
-        val out =
-          if (f.isDefinedAt(a))
-            effect(f(a))
-          else if(first)
-            effect(fallback)
-          else ()
+        // Semantically:
+        // if (f.isDefinedAt(a))
+        //   effect(f(a))
+        // else if(first)
+        //   effect(fallback)
+        // else ()
+        //
+        // But we have to use applyOrElse to prevent double evaluation:
+        var isDefinedAt = true
+        val fa = {
+          f.applyOrElse(a, (_: Any) => {
+            isDefinedAt = false
+            fallback
+          })
+        }
+        val out = if (isDefinedAt | first) effect(fa) else ()
         first = false
         out
       }

diff --git a/monadic-rx/shared/src/test/scala/mhtml/RxTests.scala b/monadic-rx/shared/src/test/scala/mhtml/RxTests.scala
@@ -411,4 +411,25 @@ class RxTests extends FunSuite {
     assert(sndList == List(-101, 1, 7))
     assert(source.isCold && sndProxy.isCold)
   }
+
+  test("no double checks with keepIf (#101)") {
+    val a = Var(0)
+    var checkCount = 0
+    val foo = a.keepIf { i =>
+      checkCount += 1
+      i > 0
+    }(0)
+
+    val cc = foo.impure.run(_ => ())
+
+    a := 1
+    a := 2
+    a := 3
+    a := 4
+
+    cc.cancel
+
+    assert(checkCount == 5)
+    assert(a.isCold)
+  }
 }