consider a "function wrapper" utility

[getify]

consider a "function wrapper" utility

[getify]

Wondering what style of function/callback to assume for wrapping, or how to accommodate multiple styles.

For example, will we be wrapping:

// expects error-first style callback
function A(cb) {
   // success: cb( .. )
   // error: cb( err )
}

// expects split callbacks
function B(success,error) {
   // success: success( .. )
   // error: error( err )
}

// expects normal success callback, throws on error
function C(success) {
   // success: success( .. )
   // error: throw err
}

Then there's the variation... should it assume the callback(s) are the first parameter(s) or the last parameter(s).

function D( val1, val2, cb ) { /* .. */ }
function E( cb, val1, val2 ) { /* .. */ }

So, how can we effectively make one utility able to wrap all these different styles?

[getify]

For example:

// error-first cb as first param
ASQ.wrapErrFCB = function wrapped(fn) {
   return function() {
      var sq = ASQ();
      fn.apply(null,[ sq.errfcb() ].concat.call(arguments));
      return sq;
   };
};

// OR

// error-first cb as last param
ASQ.wrapErrFCB = function wrapped(fn) {
   return function() {
      var sq = ASQ();
      fn.apply(null,[].slice.call(arguments).concat([ sq.errfcb() ]));
      return sq;
   };
};

Then:

function doCoolThing(cb) {
   // success: cb( null , .. )
   // error: cb( err )
}

doCoolThing = ASQ.wrapErrFCB( doCoolThing );

doCoolThing()
.val(function(result){
   result; // cool result
});

[getify]

One option is to just provide a single plugin that adds all options, perhaps on a sub-namespace so as to not pollute the ASQ namespace, like:

doCoolThing = ASQ.wrapFn.errfcb_first( doCoolThing );
// vs.
doCoolThing = ASQ.wrapFn.errfcb_last( doCoolThing );
// etc

Since they're all tiny, wouldn't be such a big deal to provide them altogether. Only major issue is naming. What to call each variation? What convention should we use for those names?

[getify]

Now I'm thinking something a bit different:

// all six (reasonable) variations
function f1(p1,p2,errfcb) {..}
function f2(errfcb,p1,p2) {..}
function f3(p1,p2,successCB,errorCB) {..}
function f4(successCB,errorCB,p1,p2) {..}
function f5(p1,p2,simpleCB) {..}
function f6(simpleCB,p1,p2) {..}

f1 = ASQ.wrap( f1 ); // assumes: ERRFCB | PARAMS_FIRST
f2 = ASQ.wrap( f2, ASQ.PARAMS_LAST ); // assumes: ERRFCB
f3 = ASQ.wrap( f3, ASQ.SPLITCB ); // assumes: PARAMS_FIRST
f4 = ASQ.wrap( f4, ASQ.SPLITCB | ASQ.PARAMS_LAST );
f5 = ASQ.wrap( f5, ASQ.SIMPLECB ); // assumes: PARAMS_FIRST
f6 = ASQ.wrap( f6, ASQ.SIMPLECB | ASQ.PARAMS_LAST );

[jaredly]

I think that convention over configurability is better here, especially when it's so easy to create your own closure and conform to whatever the lib (in this case asynqueue) expects. using bit flags seems symptomatic of too much complexity.
I'd say pick a pattern and stick with it.

[jaredly]

although, you could argue that "the pattern" is ERRFCB | PARAMS_FIRST, and you're still providing a way to jump out of that, which seems reasonable. nvm, I'm convinced.

[getify]

it's so easy to create your own closure and conform to whatever the lib (in this case asynquence) expects

I'm actually trying to do the opposite, which is to make it easy for people to adapt asynquence to other various non-standard function signatures' expectations. :)

you could argue that "the pattern" is ERRFCB | PARAMS_FIRST

Exactly, I picked that as the default because I think it's probably more common these days, but I also want to make it easy for people to adapt to other function signatures if they happen to be forced into that.

IOW, if you are in control of your function signatures, you probably don't need a wrapper in the first place, as you'll create a promise/sequence returning function from the start. But if you are forced to adapt to someone else's function signatures, which you probably don't like, I want asynquence to help you smooth that over without too much work on your part.

[ericelliott]

If I understand right it looks like you're considering ad hoc function polymorphism to interpret user intent. Risky.

The other alternatives are separate functions for each of option passing. Both of those approaches add cognitive load to your API and may only save the user a few keystrokes, once in a while.

I'd assume Node style err first, cb last and wouldn't worry about other styles. Adaptation is not hard.

Polymorhpism in this case is hazardous, IMO.

If you get too fancy you risk creating an API that is too complicated or too magical (hidden magic violates the principal of least astonishment).

Right? Or am I missing something?

[getify]

you're considering ad hoc function polymorphism to interpret user intent.

I don't think so, exactly. wrap(..) generates a new function that wraps the user-provided function, so they can from then on use the generated function in place of the original. It's kind of like the bind(..) utility in that sense.

The purpose of the bitmask options is to tell the wrap(..) utility what type of wrapper function to generate, based on the six possible function signature variations I identified (which I think cover the cases).

So I'm not trying to interpret or infer user-intent, I'm trying to let them explicitly declare what type of function they want generated.

It's not polymorphic because wrap(..) generates a new specific function according to the signature options, not a single function that polymorphs itself to all signatures.

The other alternatives are separate functions for each of option passing

That was the original idea. But six function names not only pollutes the API quite a bit, but it also presents a major naming issue.

The whole purpose of this part of the API is to provide helpers that make it so you can easily use asynquence regardless of what function signatures you're being forced to use. I have seen all six of these variations in the wild. I have no idea which variation(s) some arbitrary developer will be forced to deal with. If I'm in the business of trying to provide helpers for them, shouldn't I provide all the reasonable helpers they'll need?

only save the user a few keystrokes

If I provide only one or a couple variations, arbitrarily, and you're the user who only has to deal with any of the un-provided variations, the helpers I did provide do not in any way compose for, or aide in, your solution, so you're basically forced to do your own wrapping from scratch. A lot of developers don't know exactly how to do such things. Relatively speaking, that's a lot of cognitive load for developers (which makes it harder to adopt asynquence).

It's not just a different function call, it's actually that you have to make your own wrapper generator for each variation you need. Each is 5-10 lines of code, with tricky issues like making sure you handle parameters properly, etc.

I think that's why provided wrapper generators like these are so popular in async/promise libs in the first place.

Right?

hidden magic violates the principal of least astonishment

Is it hidden/magic? It doesn't seem so to me. It is an explicit way to specify what kind of function you want generated.

My inspiration is PHP's preg_xxx functions, which let you specify options to control regular expression behavior using bitmasks. That always seemed quite explicit to me.

The only hidden/magic is the default cases. We could remove the defaults and force you to always pass a bitmask, but that seems like a net loss.

[getify]

The other approach (which is certainly more idiomatic for JS) is an options object parameter:

// all six (reasonable) variations
function f1(p1,p2,errfcb) {..}
function f2(errfcb,p1,p2) {..}
function f3(p1,p2,successCB,errorCB) {..}
function f4(successCB,errorCB,p1,p2) {..}
function f5(p1,p2,simpleCB) {..}
function f6(simpleCB,p1,p2) {..}

f1 = ASQ.wrap( f1 ); // assumes: errfcb:true, params_first:true
f2 = ASQ.wrap( f2, { params_last:true } ); // assumes: errfcb:true
f3 = ASQ.wrap( f3, { splitcb:true } ); // assumes: params_first:true
f4 = ASQ.wrap( f4, { splitcb:true, params_last:true } );
f5 = ASQ.wrap( f5, { simplecb:true } ); // assumes params_first:true
f6 = ASQ.wrap( f6, { simplecb:true, params_last:true } );

While that's definitely more common in JS, I don't think it's necessarily better/easier than the bitmasks. Shrugs.

[jaredly]

It would certainly look less out of place in a typical node application

[garvincasimir]

A lot of developers don't know exactly how to do such things. Relatively speaking, that's a lot of cognitive load for developers (which makes it harder to adopt asynquence).

@getify if your priority is low cognitive load and adoption by as many developers as possible then it is probably best to go with the method that is most recognizable by javascript developers. I would say that is the object syntax.

[getify]

Thanks for the feedback, folks! So far, I've implemented and am testing the options-object syntax from above. I like it.

Open to more feedback to sway me in some other direction, but I'm leaning heavily that way at present.

[getify]

Thanks to all who provided feedback. I've landed ASQ.wrap(..), as documented here. Excited about how this turned out. :)