-
Notifications
You must be signed in to change notification settings - Fork 67
Add forkAll combinator #45
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,61 @@ exports._forkAff = function (nonCanceler, aff) { | |
}; | ||
} | ||
|
||
exports._forkAll = function (nonCanceler, foldl, affs) { | ||
var voidF = function(){}; | ||
|
||
return function(success, error) { | ||
var cancelers = foldl(function(acc) { | ||
return function(aff) { | ||
acc.push(aff(voidF, voidF)); | ||
return acc; | ||
} | ||
})([])(affs); | ||
|
||
var canceler = function(e) { | ||
return function(success, error) { | ||
var cancellations = 0; | ||
var result = false; | ||
var errored = false; | ||
|
||
var s = function(bool) { | ||
cancellations = cancellations + 1; | ||
result = result || bool; | ||
|
||
if (cancellations === cancelers.length && !errored) { | ||
try { | ||
success(result); | ||
} catch (e) { | ||
error(e); | ||
} | ||
} | ||
}; | ||
|
||
var f = function(err) { | ||
if (!errored) { | ||
errored = true; | ||
error(err); | ||
} | ||
}; | ||
|
||
for (var i = 0; i < cancelers.length; i++) { | ||
cancelers[i](e)(s, f); | ||
} | ||
|
||
return nonCanceler; | ||
}; | ||
}; | ||
|
||
try { | ||
success(canceler); | ||
} catch(e) { | ||
error(e); | ||
} | ||
|
||
return nonCanceler; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to be able to cancel these by propagating the canceling behavior into the forked computations, rather than returning a non-canceler here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, so to make sure I'm getting it right: Collect all the cancellers in the fold, then return a canceller that propagates to all those cancellers. Should I also push that array of cancellers to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A canceller is, I believe, an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get what the canceller does, and yes, it's very similar to what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, cancelers have a monoid, right? So you could just pass append in and smash them all together. That'd be easier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does, which is very convenient, but doesn't preserve the stack-safe property since it uses the Applicative instance for Aff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you're only going to need cancelers with truly async computations, so maybe its not a big deal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. |
||
}; | ||
} | ||
|
||
exports._makeAff = function (cb) { | ||
return function(success, error) { | ||
return cb(function(e) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah. I don't love it either :/
I might be able to extract out a helper to share with cancelWith.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I can see it's fine, it's just the mutating of the array in the fold while constructing it that made me look twice. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Thought you were referring to the sheer length of this function. Hard to tell in email sometimes