Implement FantasyLand ChainRec #125
Conversation
If feature is not resolved synchronously then old implementation produces strange fails. this way we could express equality of async Features. Also use deepEqual instead of equal.
|
Hey @buzzdecafe now you can do a review 🎉 |
|
thanks -- there's a lot for me to digest here. I don't know how much time I have to review this weekend. I hope some other contributors may be able to have a look? |
| function(v) { | ||
| return v; | ||
| }, | ||
| f(util.chainRecNext, util.chainRecDone, state.value) |
There was a problem hiding this comment.
We could make use of the existing Either to handle next and done here.
I think something like this should work:
Either.chainRec = Either.prototype.chainRec = function(f, i) {
var updateState = Either.either(
compose(Right, Left),
Either.either(compose(Left, Right), compose(Right, Right))
);
var state = Left(Right(i));
while (Either.isLeft(state)) {
state = updateState(f(Left, Right, state.value.value));
}
return state.value;
};There was a problem hiding this comment.
I have updated my implementation but I think it's better to use custom structure for representing state instead of Either and it's also simpler to understand.
|
This is great work @safareli! I can take a look into adding support for State/T and Reader/T. |
|
@scott-christopher thanks, that would be awesome! |
|
apart from some detail-y stuff in the tests, LGTM |
|
@buzzdecafe I think you have accidentally closed it :p |
|
oops, sorry. it's been a long few days |
| }, 100000))); | ||
| }); | ||
|
|
||
| it('fail Immediately', function() { |
There was a problem hiding this comment.
i'd prefer if these it statements read like English, e.g. it("responds to failure immediately") etc.
| return cTest.equivalence(Future, predicate, done, next, initial); | ||
| }); | ||
|
|
||
| it('sync and async Futures', function() { |
There was a problem hiding this comment.
a more descriptive it here would be nice. what does this test prove?
There was a problem hiding this comment.
fixed, it tests if it works when mixing sync and async futures
|
🐪 |
|
sweet 🍓 |
|
Could we bump version? |
|
v0.7.0 has been published. |
Implemented ChainRec for:
Not implemented for:
(As I don't quite understand Transformers, may create other PR when I get them)
Fixes #124