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.

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

.call alias for .map(_.wrapCallback).invoke('call') #153

Closed
LewisJEllis opened this issue Nov 22, 2014 · 3 comments
Closed

.call alias for .map(_.wrapCallback).invoke('call') #153

LewisJEllis opened this issue Nov 22, 2014 · 3 comments

Comments

@LewisJEllis
Copy link
Collaborator

.map(_.wrapCallback).invoke('call) has become a rather common pattern for me in refactoring from async to Highland, I think enough to consider making an alias, perhaps .call(). It could also potentially take an args list, deal with this nicely, etc.

Beyond convenience, this would also make it easier to draw parallels between some of async's control flow functions and Highland, much like caolan's first comment in #79 does for some of async's collection functions. Some examples:

.call().series() to async.series()
.call().parallel() to async.parallel()
.call(args) to async.applyEach(..., args)
.call(args).series() to async.applyEachSeries(..., args)

As a bonus, we can just use Function.prototype.bind() much like async uses async.apply:

async.parallel([
    async.apply(fs.writeFile, 'testfile1', 'test1'),
    async.apply(fs.writeFile, 'testfile2', 'test2'),
]);

_([
  fs.writeFile.bind(null, 'testFile1', 'test1')
  fs.writeFile.bind(null, 'testFile2', 'test2')
]).call().parallel()

This stays nice when we actually need to use bind, while async gets messy:

async.series([
  async.apply(collection.addElement.bind(collection, element))
  async.apply(dbRow.save.bind(dbRow)),
  async.apply(jobManager.enqueue.bind(jobManager, job))
]);

_([
  collection.addElement.bind(collection, element)
  dbRow.save.bind(dbRow),
  jobManager.enqueue.bind(jobManager, job)
]).call().series().resume();

This is really mostly because we have _.wrapCallback doing part of what async.apply does; we could always map async.apply similarly, but that gets long.

See also this SO post, where .call would've come in handy or even avoided the question entirely. The example from it:

function firstThing(state, next) {
  state.firstThingDone = true;
  setImmediate(next);
}

function secondThing(state, next) {
  state.secondThingDone = true;
  setImmediate(next);
}

var state = {};

async.applyEachSeries([
  firstThing,
  secondThing
], state, function (error) {
  console.log(error, state);
});

lines straight up with:

_([firstThing, secondThing])
  .call().series().toArray(function() {
    console.log(state)
  });

(don't mind the .toArray where .done would fit)

Of course, I'm not looking to just port over async functions, but I think this one addition might go a long way, especially in making it easier for async people to pick up Highland.

LewisJEllis added a commit to LewisJEllis/highland that referenced this issue Nov 27, 2014
@LewisJEllis
Copy link
Collaborator Author

Ignore the first commit; refer to the PR.

@LewisJEllis
Copy link
Collaborator Author

Closed by #171

@LewisJEllis
Copy link
Collaborator Author

@jeromew can we cut a new release with this (and the other recent additions)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants