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

react: Update tests to v2.0.0 #1346

Merged
merged 8 commits into from
Apr 20, 2018
Merged

react: Update tests to v2.0.0 #1346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bc89832
react: re-implement according to canonical data
mrcfps Feb 22, 2018
307b3f1
react: update tests to v1.2.0
mrcfps Feb 28, 2018
4d1eedb
react: correct test method name
mrcfps Feb 28, 2018
513bc6a
react: fix example solution py2-compatibility
mrcfps Feb 28, 2018
abb7e0a
react: refactor callbacks into pure functions
mrcfps Mar 3, 2018
42eccbf
Merge upstream/master into react-1257
mrcfps Feb 26, 2018
688a30c
react: update tests to v2.0.0
mrcfps Apr 20, 2018
71214fb
Merge branch 'master' into react-1257
cmccandless Apr 20, 2018
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
21 changes: 8 additions & 13 deletions exercises/react/example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from collections import defaultdict


class Cell(object):
def __init__(self):
self._watchers = []
Expand Down Expand Up @@ -31,6 +34,7 @@ def __init__(self, inputs, compute_function):
self.inputs = inputs
self.func = compute_function
self.callbacks = set()
self.callback_values = defaultdict(list)
self.compute()
self._register_inputs()

Expand All @@ -46,7 +50,7 @@ def compute(self):
if new_val != self._value:
self.value = new_val
for cb in self.callbacks:
cb.add_value(new_val)
self.callback_values[cb].append(cb(new_val))

def add_callback(self, callback):
self.callbacks.add(callback)
Expand All @@ -55,16 +59,7 @@ def remove_callback(self, callback):
if callback in self.callbacks:
self.callbacks.remove(callback)


class Callback(object):
def __init__(self):
self._values = []

def add_value(self, value):
self._values.append(value)

@property
def values(self):
results = self._values[:]
self._values = []
def expect_callback_values(self, callback):
results = self.callback_values[callback]
del self.callback_values[callback]
return results
6 changes: 2 additions & 4 deletions exercises/react/react.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,5 @@ def add_callback(self, callback):
def remove_callback(self, callback):
pass


class Callback(object):
def __init__(self):
self.values = None
def expect_callback_values(self, callback):
pass
66 changes: 43 additions & 23 deletions exercises/react/react_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import unittest

from react import InputCell, ComputeCell, Callback
from react import InputCell, ComputeCell


# Tests adapted from `problem-specifications//canonical-data.json` @ v1.2.0
Expand Down Expand Up @@ -52,43 +52,55 @@ def test_compute_cells_can_depend_on_other_compute_cells(self):
def test_compute_cells_fire_callbacks(self):
input_ = InputCell(1)
output = ComputeCell([input_], lambda inputs: inputs[0] + 1)
callback1 = Callback()

def callback1(value):
return value

output.add_callback(callback1)
input_.value = 3
self.assertEqual(callback1.values, [4])
self.assertEqual(output.expect_callback_values(callback1), [4])
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that I like expect_callback_values being a member of ComputeCell...

What do you think about something like this?

callback_buffer = []
output.add_callback(callback_buffer.append)
input_.value = 3
self.assertEqual(callback_buffer, [4])

Copy link
Author

Choose a reason for hiding this comment

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

You mean convert callback to callback_buffer, and keep outputs of callbacks in the buffer? If so, I guess problem specification of this exercise should be changed, too.

My justification for expect_callback_values being a member of ComputeCell is that ComputeCell seems to be acting as a host for callbacks to be attached to and detached from. Or you can imagine callbacks as plugins for the ComputeCell, so trying to obtain output values of a callback is like reading the state of a plugin.

However, these are just analogies. Insights are welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are discussions on rewording the canonical-data, but I don't think that semantic changes are necessary to it.

Or you can imagine callbacks as plugins for the ComputeCell, so trying to obtain output values of a callback is like reading the state of a plugin.

Close: this is more of a publisher/subscriber scenario. The ComputerCell "publishes" new values by calling the registered callbacks. The callback is the subscription mechanism. The publisher doesn't care what the subscriber object is or how it uses the published data, and has no access to it. In a practical use case, if one were trying to use the solution for this exercise to perform calculations, they might simply use output.add_callback(lambda v: print(v))

I don't remember if I linked @petertseng's comment before (exercism/problem-specifications#1194 (comment)), but it's a good in-context note on what a callback is/should be.


def test_callbacks_only_fire_on_change(self):
input_ = InputCell(1)
output = ComputeCell(
[input_],
lambda inputs: 111 if inputs[0] < 3 else 222
)
callback1 = Callback()

def callback1(value):
return value

output.add_callback(callback1)
input_.value = 2
self.assertEqual(callback1.values, [])
self.assertEqual(output.expect_callback_values(callback1), [])
input_.value = 4
self.assertEqual(callback1.values, [222])
self.assertEqual(output.expect_callback_values(callback1), [222])

def test_callbacks_do_not_report_already_reported_values(self):
input_ = InputCell(1)
output = ComputeCell([input_], lambda inputs: inputs[0] + 1)
callback1 = Callback()

def callback1(value):
return value

output.add_callback(callback1)
input_.value = 2
self.assertEqual(callback1.values, [3])
self.assertEqual(output.expect_callback_values(callback1), [3])
input_.value = 3
self.assertEqual(callback1.values, [4])
self.assertEqual(output.expect_callback_values(callback1), [4])

def test_callbacks_can_be_added_and_removed(self):
input_ = InputCell(11)
output = ComputeCell([input_], lambda inputs: inputs[0] + 1)
callback1 = Callback()
callback2 = Callback()
callback3 = Callback()

def callback1(value):
return value

def callback2(value):
return value

def callback3(value):
return value

output.add_callback(callback1)
output.add_callback(callback2)
Expand All @@ -97,17 +109,21 @@ def test_callbacks_can_be_added_and_removed(self):
output.add_callback(callback3)
input_.value = 41

self.assertEqual(callback1.values, [32])
self.assertEqual(callback2.values, [32, 42])
self.assertEqual(callback3.values, [42])
self.assertEqual(output.expect_callback_values(callback1), [32])
self.assertEqual(output.expect_callback_values(callback2), [32, 42])
self.assertEqual(output.expect_callback_values(callback3), [42])

def test_removing_a_callback_multiple_times(self):
"""Guard against incorrect implementations which store their
callbacks in an array."""
input_ = InputCell(1)
output = ComputeCell([input_], lambda inputs: inputs[0] + 1)
callback1 = Callback()
callback2 = Callback()

def callback1(value):
return value

def callback2(value):
return value

output.add_callback(callback1)
output.add_callback(callback2)
Expand All @@ -116,8 +132,8 @@ def test_removing_a_callback_multiple_times(self):
output.remove_callback(callback1)
input_.value = 2

self.assertEqual(callback1.values, [])
self.assertEqual(callback2.values, [3])
self.assertEqual(output.expect_callback_values(callback1), [])
self.assertEqual(output.expect_callback_values(callback2), [3])

def test_callbacks_should_only_be_called_once(self):
"""Guard against incorrect implementations which call a callback
Expand All @@ -130,11 +146,13 @@ def test_callbacks_should_only_be_called_once(self):
[plus_one, minus_one2],
lambda inputs: inputs[0] * inputs[1]
)
callback1 = Callback()

def callback1(value):
return value

output.add_callback(callback1)
input_.value = 4
self.assertEqual(callback1.values, [10])
self.assertEqual(output.expect_callback_values(callback1), [10])

def test_callbacks_not_called_so_long_as_output_not_changed(self):
"""Guard against incorrect implementations which call callbacks
Expand All @@ -146,14 +164,16 @@ def test_callbacks_not_called_so_long_as_output_not_changed(self):
[plus_one, minus_one],
lambda inputs: inputs[0] - inputs[1]
)
callback1 = Callback()

def callback1(value):
return value

always_two.add_callback(callback1)
input_.value = 2
input_.value = 3
input_.value = 4
input_.value = 5
self.assertEqual(callback1.values, [])
self.assertEqual(always_two.expect_callback_values(callback1), [])


if __name__ == '__main__':
Expand Down