Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

How is Future.wrap supposed to be used? #127

Closed
fresheneesz opened this Issue · 5 comments

2 participants

@fresheneesz

It seems like you use Future.wrap like this:

var f = Future.wrap(someAsyncFunction, 2)('a', 'b');

If I don't specify idx, running it always gives the error: "function expects no more than -1 arguments". This seems wrong...

But why require specifying the number of parameters in the first place? Couldn't you rewrite like this without any real downside?:

function(fn) {
  return function() {
    var args = Array.prototype.slice.call(arguments);
    var future = new Future;
    args.push(future.resolver());
    fn.apply(this, args);
    return future;
  };
};

I wrote a function I like a little better:

// either used like futureWrap(function(){ ... })(arg1,arg2,etc) or
//  futureWrap(object, 'methodName')(arg1,arg2,etc)
Future.wrap = function() {
    // function
    if(arguments.length === 1) {
        var fn = arguments[0];
        var object = undefined;

    // object, function
    } else {
        var object = arguments[0];
        var fn = object[arguments[1]];
    }

    return function() {
        var args = Array.prototype.slice.call(arguments);
        var future = new Future;
        args.push(future.resolver());
        var me = this;
        if(object) me = object;
        fn.apply(me, args);
        return future;
    };
};

That way you can also call object methods pretty directly:

var db = mongo.db('localhost/local', {journal: true});
Future.wrap(db.collection('user').find(), 'toArray')().wait()

Thoughts?

@fresheneesz

@laverdet Please comment

@laverdet
Owner

If I don't specify idx, running it always gives the error: "function expects no more than -1 arguments". This seems wrong...

If you get this message it means the .length property on someAsyncFunction is incorrect.

The method you posted where the callback is just appended to the function parameters is probably better in the long run.

@fresheneesz

Is that something you'd like me to create a pull request for?

@laverdet
Owner

Sure

@fresheneesz fresheneesz referenced this issue from a commit in fresheneesz/node-fibers
bt fixing the wrap function laverdet#127 3392984
@laverdet
Owner

A new version of wrap was committed in 969e81d

@laverdet laverdet closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.