Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@
"difficulty": 1,
"slug": "change",
"topics": []
},
{
"difficulty": 10,
"slug": "react",
"topics": []
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this track want to add any topics? I suggest a similar list as those in Ceylon https://github.com/exercism/xceylon/blob/master/config.json#L52

Reactive programming, generics, nested classes, higher-order functions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yea, would be great to fill them in where we can.

}
]
}
28 changes: 28 additions & 0 deletions exercises/react/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
buildscript {
ext.kotlin_version = '1.0.6'
repositories {
mavenCentral()
}
dependencies {
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
}
}

apply plugin: 'kotlin'

repositories {
mavenCentral()
}

dependencies {
compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"

testCompile 'junit:junit:4.12'
testCompile "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
}
test {
testLogging {
exceptionFormat = 'full'
events = ["passed", "failed", "skipped"]
}
}
65 changes: 65 additions & 0 deletions exercises/react/src/example/kotlin/React.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
class Reactor<T>() {
abstract inner class Cell {
abstract val value: T
internal val dependents = mutableListOf<ComputeCell>()
Copy link
Copy Markdown
Member Author

@petertseng petertseng Jan 18, 2017

Choose a reason for hiding this comment

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

interestingly, protected is not possible, because then the c.dependents.add(this) in the constructors all fail compilation with:

:compileKotlin
e: /home/pt/xkotlin/exercises/react/src/main/kotlin/React.kt: (22, 15): Cannot access 'dependents': it is protected in 'Cell'
e: /home/pt/xkotlin/exercises/react/src/main/kotlin/React.kt: (25, 16): Cannot access 'dependents': it is protected in 'Cell'
e: /home/pt/xkotlin/exercises/react/src/main/kotlin/React.kt: (26, 16): Cannot access 'dependents': it is protected in 'Cell'
 FAILED

This is a little strange to me, since ComputeCell is (I hope?!) a subclass of Cell. Perhaps it doesn't apply inside constructors, or something like that... after all, you could consider the constructor to be a function not associated with any particular class, but it does return an instance of a particular class...?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yea, I'm likewise surprised that this error shows up for a protected variable when being accessed in a subclass' constructor, it is available in the subclass outside of the constructor though...

}

interface Subscription {
fun cancel()
}

inner class InputCell(initialValue: T) : Cell() {
override var value: T = initialValue
Copy link
Copy Markdown
Member Author

@petertseng petertseng Jan 22, 2017

Choose a reason for hiding this comment

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

looks like http://kotlinlang.org/docs/reference/delegated-properties.html#observable could be used here, I'll consider it, though it's understandable enough without it.

(I'll probably not use it in the PR. Maybe in my submission to exercism.io!)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, observables do seem like a great fit for this exercise - I wonder if there is a way to leverage the ObservableProperty as a super type. But what you currently have works, we should care more about if API driven by the tests make sense.

set(newValue) {
field = newValue
dependents.forEach { it.propagate() }
dependents.forEach { it.fireCallbacks() }
}
}

inner class ComputeCell private constructor(val newValue: () -> T) : Cell() {
override var value: T = newValue()
private set

private var lastCallbackValue = value
private var callbacksIssued = 0
private val activeCallbacks = mutableMapOf<Int, (T) -> Any>()

constructor(vararg cells: Cell, f: (List<T>) -> T) : this({ f(cells.map { it.value }) }) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

List seemed most appropriate since it allowed indexing into it, so the lambdas can say it[0], it[0] + it[1], etc. Using Iterable could work, but would be weird - we can do it.first and it.fold { a, b -> a + b} or something, I suppose.

for (cell in cells) {
cell.dependents.add(this)
}
}

fun addCallback(f: (T) -> Any): Subscription {
Copy link
Copy Markdown
Member Author

@petertseng petertseng Jan 18, 2017

Choose a reason for hiding this comment

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

Sigh, I changed too many intervening lines, so the old comment is on an "outdated diff" now. Gotta post it again.

I hope this is not too strange of an API. The API looks something like:

val cell: ComputeCell = ...
val sub = cell.addCallback { print(it) }
// ...
sub.cancel()

The alternative is to have addCallback return some sort of ID:

val cell: ComputeCell = ...
val subID = cell.addCallback { print(it) }
// ...
cell.removeCallback(subID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really like what you did here returning back a Subscription. Do you think we should provide some initial getting started help by at least providing some interfaces?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you think we should provide some initial getting started help by at least providing some interfaces?

I would be in support of that, because it may be otherwise difficult to determine from just the tests that addCallback is supposed to return something that you can call cancel() on. It's easy enough to read function signatures if the expected return value is just a primitive, but if it needs to conform to an interface, then help is good.

By the way, it's time for me to point out the master list of all tracks' discussions on providing stubs exercism/discussions#114

I'll let y'all decide what y'all want to do with the other exercises, but for this one, how far should we go?

  • only provide the Subscription interface, and specify that addCallback should return something that conforms to it?
  • provide Subscription, and stubs for Cell, ComputeCell, InputCell? But be careful, because this could constrain the student's implementation; maybe they don't want to have those exact three classes (maybe they just want to have a single Cell class?

I'll start with the first option and see how it goes.

val id = callbacksIssued
callbacksIssued++
activeCallbacks[id] = f
return object : Subscription {
override fun cancel() {
activeCallbacks.remove(id)
}
}
}

internal fun propagate() {
val nv = newValue()
if (nv == value) {
return
}
value = nv
dependents.forEach { it.propagate() }
}

internal fun fireCallbacks() {
if (value == lastCallbackValue) {
return
}
lastCallbackValue = value
for (cb in activeCallbacks.values) {
cb(value)
}
dependents.forEach { it.fireCallbacks() }
}
}
}
7 changes: 7 additions & 0 deletions exercises/react/src/main/kotlin/React.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Reactor<T>() {
// Your compute cell's addCallback method must return an object
// that implements the Subscription interface.
interface Subscription {
fun cancel()
}
}
221 changes: 221 additions & 0 deletions exercises/react/src/test/kotlin/ReactTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
import org.junit.Test
import org.junit.Ignore
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
import kotlin.test.assertEquals

class ReactTest {
@Test
fun inputCellsHaveValue() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(10)
assertEquals(10, input.value)
}

@Ignore
@Test
fun inputCellsValueCanBeSet() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(4)
input.value = 20
assertEquals(20, input.value)
}

@Ignore
@Test
fun computeCellsCalculateInitialValue() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val output = reactor.ComputeCell(input) { it[0] + 1 }
assertEquals(2, output.value)
}

@Ignore
@Test
fun computeCellsTakeInputsInTheRightOrder() {
val reactor = Reactor<Int>()
val one = reactor.InputCell(1)
val two = reactor.InputCell(2)
val output = reactor.ComputeCell(one, two) { it[0] + it[1] * 10 }
assertEquals(21, output.value)
}

@Ignore
@Test
fun computeCellsUpdateValueWhenDependenciesAreChanged() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val output = reactor.ComputeCell(input) { it[0] + 1 }
input.value = 3
assertEquals(4, output.value)
}

@Ignore
@Test
fun computeCellsCanDependOnOtherComputeCells() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val timesTwo = reactor.ComputeCell(input) { it[0] * 2 }
val timesThirty = reactor.ComputeCell(input) { it[0] * 30 }
val output = reactor.ComputeCell(timesTwo, timesThirty) { it[0] + it[1] }

assertEquals(32, output.value)
input.value = 3
assertEquals(96, output.value)
}

@Ignore
@Test
fun computeCellsFireCallbacks() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val output = reactor.ComputeCell(input) { it[0] + 1 }

val vals = mutableListOf<Int>()
output.addCallback { vals.add(it) }

input.value = 3
assertEquals(listOf(4), vals)
}

@Ignore
@Test
fun callbacksOnlyFireOnChange() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val output = reactor.ComputeCell(input) { if (it[0] < 3) 111 else 222 }

val vals = mutableListOf<Int>()
output.addCallback { vals.add(it) }

input.value = 2
assertEquals(listOf<Int>(), vals)

input.value = 4
assertEquals(listOf(222), vals)
}

@Ignore
@Test
fun callbacksCanBeAddedAndRemoved() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(11)
val output = reactor.ComputeCell(input) { it[0] + 1 }

val vals1 = mutableListOf<Int>()
val sub1 = output.addCallback { vals1.add(it) }
val vals2 = mutableListOf<Int>()
output.addCallback { vals2.add(it) }

input.value = 31
sub1.cancel()

val vals3 = mutableListOf<Int>()
output.addCallback { vals3.add(it) }

input.value = 41

assertEquals(listOf(32), vals1)
assertEquals(listOf(32, 42), vals2)
assertEquals(listOf(42), vals3)
}

@Ignore
@Test
fun removingACallbackMultipleTimesDoesntInterfereWithOtherCallbacks() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val output = reactor.ComputeCell(input) { it[0] + 1 }

val vals1 = mutableListOf<Int>()
val sub1 = output.addCallback { vals1.add(it) }
val vals2 = mutableListOf<Int>()
output.addCallback { vals2.add(it) }

for (i in 1..10) {
sub1.cancel()
}

input.value = 2
assertEquals(listOf<Int>(), vals1)
assertEquals(listOf(3), vals2)
}

@Ignore
@Test
fun callbacksShouldOnlyBeCalledOnceEvenIfMultipleDependenciesChange() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val plusOne = reactor.ComputeCell(input) { it[0] + 1 }
val minusOne1 = reactor.ComputeCell(input) { it[0] - 1 }
val minusOne2 = reactor.ComputeCell(minusOne1) { it[0] - 1 }
val output = reactor.ComputeCell(plusOne, minusOne2) { it[0] * it[1] }

val vals = mutableListOf<Int>()
output.addCallback { vals.add(it) }

input.value = 4
assertEquals(listOf(10), vals)
}

@Ignore
@Test
fun callbacksShouldNotBeCalledIfDependenciesChangeButOutputValueDoesntChange() {
val reactor = Reactor<Int>()
val input = reactor.InputCell(1)
val plusOne = reactor.ComputeCell(input) { it[0] + 1 }
val minusOne = reactor.ComputeCell(input) { it[0] - 1 }
val alwaysTwo = reactor.ComputeCell(plusOne, minusOne) { it[0] - it[1] }

val vals = mutableListOf<Int>()
alwaysTwo.addCallback { vals.add(it) }

for (i in 1..10) {
input.value = i
}

assertEquals(listOf<Int>(), vals)
}
}

@RunWith(Parameterized::class)
// This is a digital logic circuit called an adder:
// https://en.wikipedia.org/wiki/Adder_(electronics)
class ReactAdderTest(val input: Input, val expected: Expected) {

companion object {
data class Input(val a: Boolean, val b: Boolean, val carryIn: Boolean)
data class Expected(val carryOut: Boolean, val sum: Boolean)

@JvmStatic
@Parameterized.Parameters(name = "{index}: {0} = {1}")
fun data() = listOf(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All of these parameters make it a bit difficult to read, perhaps something like this would be an improvement:

diff --git a/exercises/react/src/test/kotlin/ReactTest.kt b/exercises/react/src/test/kotlin/ReactTest.kt
index 92cc9e0..a514b6b 100644
--- a/exercises/react/src/test/kotlin/ReactTest.kt
+++ b/exercises/react/src/test/kotlin/ReactTest.kt
@@ -169,28 +169,32 @@ class ReactTest {
 @RunWith(Parameterized::class)
 // This is a digital logic circuit called an adder:
 // https://en.wikipedia.org/wiki/Adder_(electronics)
-class ReactAdderTest(val aval: Boolean, val bval: Boolean, val cin: Boolean, val expectedCout: Boolean, val expectedSum: Boolean) {
+class ReactAdderTest(val input: Input, val expected: Expected) {
+
     companion object {
+        data class Input(val a: Boolean, val b: Boolean, val carryIn: Boolean)
+        data class Expected(val carryOut: Boolean, val sum: Boolean)
+
         @JvmStatic
-        @Parameterized.Parameters(name = "{index}: {0} + {1} + {2} = {3} {4}")
+        @Parameterized.Parameters(name = "{index}: {0} = {1}")
         fun data() = listOf(
-                arrayOf(false, false, false, false, false),
-                arrayOf(false, false, true, false, true),
-                arrayOf(false, true, false, false, true),
-                arrayOf(false, true, true, true, false),
-                arrayOf(true, false, false, false, true),
-                arrayOf(true, false, true, true, false),
-                arrayOf(true, true, false, true, false),
-                arrayOf(true, true, true, true, true)
+                arrayOf(Input(a=false, b=false, carryIn=false), Expected(carryOut=false, sum=false)),
+                arrayOf(Input(a=false, b=false, carryIn=true),  Expected(carryOut=false, sum=true)),
+                arrayOf(Input(a=false, b=true,  carryIn=false), Expected(carryOut=false, sum=true)),
+                arrayOf(Input(a=false, b=true,  carryIn=true),  Expected(carryOut=true,  sum=false)),
+                arrayOf(Input(a=true,  b=false, carryIn=false), Expected(carryOut=false, sum=true)),
+                arrayOf(Input(a=true,  b=false, carryIn=true),  Expected(carryOut=true,  sum=false)),
+                arrayOf(Input(a=true,  b=true,  carryIn=false), Expected(carryOut=true,  sum=false)),
+                arrayOf(Input(a=true,  b=true,  carryIn=true),  Expected(carryOut=true,  sum=true))
         )
     }
 
     @Test
     fun test() {
         val reactor = Reactor<Boolean>()
-        val a = reactor.InputCell(aval)
-        val b = reactor.InputCell(bval)
-        val carryIn = reactor.InputCell(cin)
+        val a = reactor.InputCell(input.a)
+        val b = reactor.InputCell(input.b)
+        val carryIn = reactor.InputCell(input.carryIn)
 
         val aXorB = reactor.ComputeCell(a, b) { a, b -> a.xor(b) }
         val sum = reactor.ComputeCell(aXorB, carryIn) { axorb, cin -> axorb.xor(cin) }
@@ -199,10 +203,6 @@ class ReactAdderTest(val aval: Boolean, val bval: Boolean, val cin: Boolean, val
         val aAndB = reactor.ComputeCell(a, b) { a, b -> a && b }
         val carryOut = reactor.ComputeCell(aXorBAndCin, aAndB) { a, b -> a || b }
 
-        // Test them both at once so if they fail we get to see both values.
-        assertEquals(
-                listOf(expectedSum, expectedCout),
-                listOf(sum.value, carryOut.value)
-        )
+        assertEquals(expected, Expected(sum=sum.value, carryOut=carryOut.value))
     }
 }

Copy link
Copy Markdown
Member Author

@petertseng petertseng Feb 9, 2017

Choose a reason for hiding this comment

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

All of these parameters make it a bit difficult to read, perhaps something like this would be an improvement

So, I hope you don't mind if I just take that exactly as-is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good to me ;)

arrayOf(Input(a=false, b=false, carryIn=false), Expected(carryOut=false, sum=false)),
arrayOf(Input(a=false, b=false, carryIn=true), Expected(carryOut=false, sum=true)),
arrayOf(Input(a=false, b=true, carryIn=false), Expected(carryOut=false, sum=true)),
arrayOf(Input(a=false, b=true, carryIn=true), Expected(carryOut=true, sum=false)),
arrayOf(Input(a=true, b=false, carryIn=false), Expected(carryOut=false, sum=true)),
arrayOf(Input(a=true, b=false, carryIn=true), Expected(carryOut=true, sum=false)),
arrayOf(Input(a=true, b=true, carryIn=false), Expected(carryOut=true, sum=false)),
arrayOf(Input(a=true, b=true, carryIn=true), Expected(carryOut=true, sum=true))
)
}

@Ignore
@Test
fun test() {
val reactor = Reactor<Boolean>()
val a = reactor.InputCell(input.a)
val b = reactor.InputCell(input.b)
val carryIn = reactor.InputCell(input.carryIn)

val aXorB = reactor.ComputeCell(a, b) { it[0].xor(it[1]) }
val sum = reactor.ComputeCell(aXorB, carryIn) { it[0].xor(it[1]) }

val aXorBAndCin = reactor.ComputeCell(aXorB, carryIn) { it[0] && it[1] }
val aAndB = reactor.ComputeCell(a, b) { it[0] && it[1] }
val carryOut = reactor.ComputeCell(aXorBAndCin, aAndB) { it[0] || it[1] }

assertEquals(expected, Expected(sum=sum.value, carryOut=carryOut.value))
}
}
1 change: 1 addition & 0 deletions exercises/settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ include 'pascals-triangle'
include 'phone-number'
include 'pig-latin'
include 'raindrops'
include 'react'
include 'rna-transcription'
include 'robot-name'
include 'roman-numerals'
Expand Down