react: Add new exercise #20

petertseng · 2017-01-17T01:55:26Z

I highly recommend squashing these commits before merging, because the intermediary WIP commits do not necessarily build and are not reflective of a complete exercise.

`given Element satisfies Object` saves the day

petertseng

this was a very ambitious choice for the sixth exercise to be added to this track, and it might have been too ambitious.

There are ideas I feel like I don't yet know how to express well in the language, so it may pay well to try porting some different exercises first, shelving this attempt.

I sent this PR so that I have a record of the work, I can comment it on where I had troubles, and maybe others will help. I'll see what happens.

petertseng · 2017-01-17T01:57:35Z

exercises/react/example/Reactor.ceylon

+    variable Element currentValue_;
+    Element() newValue;
+
+    shared new single(Cell c, Element(Element) f) {


here I had a hope that a call Reactor<Integer>.ComputeCell.single would be able to treat this Element(Element) as Integer(Integer), but it does not currently seem this is true.

Because it was not true, I was forced to give up making this generic. Now our reactor only acts on Integers.

update: given Element satisfies Object was necessary.

petertseng · 2017-01-17T02:00:47Z

exercises/react/example/Reactor.ceylon

    shared new single(Cell c, Element(Element) f) {
+      c.addDependent(this);


this is not legal.

My alternatives are:

create a different function on Reactor that constructs a compute cell then adds it as a dependent of its dependencies. I was afraid it would be unidiomatic to expose this constructor-that-isn't-a-constructor, but perhaps it is not so bad.

do it a different way, do not let cells know about their dependencies (why do I do it this way? only because it's the way which I did it in other language)

force the caller to add the just-constructed ComputeCell as a dependent of its dependencies. I don't find this a reasonable interface.

petertseng · 2017-01-17T02:00:53Z

exercises/react/example/Reactor.ceylon

      newValue = () => f(c.currentValue);
      currentValue_ = newValue();
    }

+    shared new double(Cell c1, Cell c2, Element(Element, Element) f) {
+      c1.addDependent(this);


also illegal.

petertseng · 2017-01-17T02:00:56Z

exercises/react/example/Reactor.ceylon

      newValue = () => f(c.currentValue);
      currentValue_ = newValue();
    }

+    shared new double(Cell c1, Cell c2, Element(Element, Element) f) {
+      c1.addDependent(this);
+      c2.addDependent(this);


also illegal.

petertseng · 2017-01-17T02:04:22Z

closing to prevent accidental merges, but thoughts still appreciated.

petertseng · 2017-01-17T02:48:12Z

Okay, guess I have to make it so that the reactor is in charge of constructing.

petertseng

a little unsure if I should keep all the value in the tests (lets students be a bit more flexible with types if they can find a way to) or explicitly specify types (to make sure the types are as we expect)

petertseng · 2017-01-17T03:36:01Z

exercises/react/example/Reactor.ceylon

+
+alias Element => Integer;
+
+class Reactor() {


in this example solution, the reactor is absolutely useless; it does nothing. But I wanted to leave it in case a student's implementation truly needs it (for example, they store all their cells in an array and iterate over the array to propagate values)

petertseng · 2017-01-17T03:36:33Z

exercises/react/example/Reactor.ceylon

+    shared actual Element currentValue => currentValue_;
+
+    variable Integer callbacksIssued = 0;
+    variable MutableMap<Integer, Callback> activeCallbacks = HashMap<Integer, Callback>();


ouch, a bit of repetition here.

petertseng · 2017-01-17T03:58:11Z

exercises/react/example/Reactor.ceylon

+    }
+  }
+
+  shared ComputeCell newComputeCell1(Cell c, Element(Element) f) {


#20 (comment) explains why Reactor must be in charge of creating new compute cells - notice the addDependent call on the newly-created cell. If this was done in an ComputeCell initialiser, it will leak a reference to this, which is illegal.

petertseng · 2017-01-17T03:58:20Z

exercises/react/example/Reactor.ceylon

+    return cell;
+  }
+
+  shared ComputeCell newComputeCell2(Cell c1, Cell c2, Element(Element, Element) f) {


#20 (comment) explains why Reactor must be in charge of creating new compute cells - notice the addDependent call on the newly-created cell. If this was done in an ComputeCell initialiser, it will leak a reference to this, which is illegal.

same.

petertseng · 2017-01-17T03:58:56Z

exercises/react/example/Reactor.ceylon

+    }
+  }
+
+  shared InputCell newInputCell(Element initialValue) {


it's not strictly necessary to delegate the creation to a Reactor, but it is consistent with newComputeCell* functions, which are necessary because of #20 (comment).

petertseng · 2017-01-18T09:54:03Z

exercises/react/source/react/ReactorTest.ceylon

+}
+
+test
+void removingCallbackMultipleTimesDoesntInterfereWithOtherCallbacks() {


there was supposed to be another test saying that callbacks can be removed - where did it go??????

(I'll add it back, just puzzled at how it could have disappeared from under my nose)

Added back.

petertseng · 2017-01-18T17:03:17Z

I'd like to go back to being able to say reactor.ComputeCell.single and reactor.ComputeCell.double and I think I have an idea (wrap another Cell in a ComputeCell). I'll see whether it works.

petertseng · 2017-01-19T08:05:27Z

I'd like to go back to being able to say reactor.ComputeCell.single and reactor.ComputeCell.double and I think I have an idea

ah very interesting, it is at this very point where I run into:

source/react/ReactorTest.ceylon:22: error: left operand must be of summable type: 'Element' is not a subtype of 'Summable'
  value output = r.ComputeCell.single(input, (x) => x + 1);
                                                    ^
1 error

I wonder why this is. I may have to defer on this, merge it as it is now, and try again later.

petertseng · 2017-01-19T09:17:54Z

Annotating the functions passed-in makes it work.

petertseng · 2017-01-19T09:25:25Z

It pleases me more (since it seems more idiomatic Ceylon, though what would I know?) to provide this cleaner interface to students, though it complicates the implementation a bit. I will see if there is anything else that can be done.

Argh, we have to annotate types of all the compute functions now!!!

petertseng · 2017-01-20T11:05:42Z

exercises/react/example/Reactor.ceylon

+    Inner inner;
+
+    shared new single(Cell c, Element(Element) f) extends Cell() {
+      // Since `c.addDependent(this)` is illegal, we have to create an inner cell.


for the record, I understand why it's illegal (if you give someone access to your this before you've initialised, they can reference one of your uninitialised field, which is unsafe)

petertseng · 2017-01-20T11:21:11Z

Final answer.

petertseng added 6 commits January 15, 2017 15:04

react: WIP: Element isn't Summable

28a535d

react: This doesn't work either

9a82ed2

react: no longer generic

2e71650

react: Doh, leaks this reference in initializer

dc2ff7f

react: It can be generic again!

34d96e6

`given Element satisfies Object` saves the day

react: create InputCell directly

1738601

petertseng commented Jan 17, 2017

View reviewed changes

petertseng closed this Jan 17, 2017

petertseng added 2 commits January 16, 2017 18:47

react: reactor in charge of constructing

0ac2ed4

react: complete exercise

1fc494e

petertseng reopened this Jan 17, 2017

petertseng force-pushed the react branch 2 times, most recently from 18780f4 to 1b90337 Compare January 17, 2017 03:35

petertseng commented Jan 17, 2017

View reviewed changes

petertseng force-pushed the react branch from 1b90337 to eb8e11e Compare January 17, 2017 03:39

petertseng changed the title ~~WIP: react exercise~~ react: Add new exercise Jan 17, 2017

petertseng commented Jan 17, 2017

View reviewed changes

petertseng commented Jan 18, 2017

View reviewed changes

petertseng mentioned this pull request Jan 18, 2017

react: add to track exercism/kotlin#29

Merged

petertseng force-pushed the react branch from eb8e11e to 2c58d64 Compare January 18, 2017 10:50

petertseng force-pushed the react branch from d91cdaf to e69a2c1 Compare January 19, 2017 09:17

petertseng added 2 commits January 19, 2017 21:03

react: restore ComputeCell.single and double

e710442

Argh, we have to annotate types of all the compute functions now!!!

react: add adder

a0550b4

petertseng force-pushed the react branch from e69a2c1 to e710442 Compare January 20, 2017 05:04

petertseng commented Jan 20, 2017

View reviewed changes

petertseng merged commit de77cf2 into exercism:master Jan 20, 2017

petertseng deleted the react branch January 20, 2017 11:39

		shared new single(Cell c, Element(Element) f) {
		c.addDependent(this);

Uh oh!

react: Add new exercise #20

react: Add new exercise #20

Uh oh!

Conversation

petertseng commented Jan 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng commented Jan 17, 2017

Uh oh!

petertseng commented Jan 17, 2017

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng commented Jan 18, 2017

Uh oh!

petertseng commented Jan 19, 2017

Uh oh!

petertseng commented Jan 19, 2017

Uh oh!

petertseng commented Jan 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng commented Jan 20, 2017

Uh oh!

Uh oh!

petertseng commented Jan 17, 2017 •

edited

Loading

petertseng Jan 18, 2017 •

edited

Loading