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

Helper to manage function arguments #69

Closed
LeaVerou opened this issue Dec 9, 2015 · 30 comments
Closed

Helper to manage function arguments #69

LeaVerou opened this issue Dec 9, 2015 · 30 comments
Assignees

Comments

@LeaVerou
Copy link
Owner

LeaVerou commented Dec 9, 2015

This is a very frequent pattern in Bliss: A function which accepts either separate arguments or objects which provide the same values en masse.
Here are a few examples:

  • $.delegate() (2 levels of this!)
  • $.lazy()
  • $.live()
  • $.set()
  • $.once()
  • $.add()

Also, several others that might benefit from similar handling, e.g. $.events().

I was thinking, it would be nice to have a small helper to handle these cases, so that the functions themselves are only defined for the case of having everything as separate argument, and the helper takes care of looping if the argument is an object instead.
It doesn't have to handle nested cases of this, since that only happens in $.delegate(), and we could apply the helper twice there.
That way, not only the Bliss code will be simplified, but also our users get a helper function that might help them on their own code too.
I spent some time thinking about this before releasing Bliss, but couldn't come up with a good idea for a good API for such a helper. Let’s brainstorm in this thread if you think it's worthwhile!

@dperrymorrow
Copy link
Collaborator

If I am understanding you correctly, underscore does something like this with an internal function to process arguments for functions that support "overloading".

https://github.com/jashkenas/underscore/blob/master/underscore.js#L105

Is that what you are getting at? Or something different?

@LeaVerou
Copy link
Owner Author

LeaVerou commented Dec 9, 2015

No, not at all, although what I'm referring to is indeed (a subset of) overloading.
What I meant was, there are many functions that have two signatures:

function (key, value)
function (obj) // Multiple key-value pairs

and the implementation is one of the following two general themes (with many variations, especially on the checks):

// Object centric approach
function foo(obj) {
    if (arguments.length == 2) {
        obj = {};
        obj[arguments[0]] = arguments[1];
    }

    for (var key in obj) {
        value = obj[key];
        // [actual function logic]
    }
}
// Argument centric approach
function foo(key, value) {
    if (arguments.length == 2) {
        // [actual function logic]
    }
    else {
        for (var key in obj) {
            foo(key, obj[key]);
        }
    }
}

@dperrymorrow
Copy link
Collaborator

ok, I see, thanks for explaining further.
I did work with this, i have some tests around this feature for $.lazy

Ill think about this.

@brushed
Copy link

brushed commented Dec 10, 2015

What about using an overload helper function, taking care of the for...in loop ?

function overloadObjectArgument( foo ){

    return function( key_or_object, value ){

        var obj = ( arguments.length == 2 ) ? { key_or_object : value } : key_or_object;

        for( var key in obj ){

            foo( key, obj[key] ); 

        }
    };
}

var foo1 = overloadObjectArgument( function(key, value){
    // actual function logic
})();
var foo2 = overloadObjectArgument( function(key, value){
    // actual function logic
})();

@dperrymorrow
Copy link
Collaborator

yeah something like that, or perhaps not a callback function and it just takes the args and returns an object;

lazy: function(obj, property, getter) {
  var keyVal = processArgs(arguments);
  for(var key in keyVal) {
    // ...
  }
}

I'm working to finish the tests around the mentioned methods above so that when this is refactored to use a helper we would have coverage to ensure its working as expected.

@LeaVerou
Copy link
Owner Author

Nice work, everybody!
I would rather use the argument-centric approach in the actual functions, instead of repeating the for..in loop every time.
What @brushed suggested is closer to what I had in mind (we need to bikeshed the name though, I strongly favor names with only 1 word).
One issue is that there aren't always 2 arguments, sometimes there are other arguments before those, so it probably needs an index that defaults to the case where they are the only arguments.
Also, it needs to be possible to apply it twice, to work for things like $.delegate().
Another potential issue is what happens with the return value. I think all the methods that use it though are void, in which case it's not a problem.

@dperrymorrow
Copy link
Collaborator

ok so,

  • the helper method will handle the iteration on keys
  • needs a short concise name
  • needs to handle variable number of items in the arguments Array
  • needs to take an index for parameter to start on
  • needs to be useable multiple times for $.delegate

@LeaVerou
Copy link
Owner Author

No 3 is not as liberal as that. Usually the arguments before it are of a fixed number, so we could provide an index as an argument.
Also, while I like one word names, it seems very difficult for something like this. But let's refrain from superLongJavaStyleNames() :)

@brushed
Copy link

brushed commented Dec 12, 2015

We can cope with a variable number of fixed arguments by using "foo.length" which returns the declared number of arguments, and compare it to the passed arguments.

I've used map() in case of multiple invocations, so it's possible to capture the return values, if needed.
FFS: this may not be smart e.g. in case of chainable invocations.

Unfortunately, the current implementation of $.add() cannot be generated with this helper, because in this case the foldable arguments are followed by 2 more optional arguments. Considering the complexity of $.add, I would suggest to simplify the implementation and only support objects as first parameter.

//unfoldArgs(fn) 
//    Returns a function which accepts a variable number of fixed parameters, 
//    followed by either separate key/value arguments or object arguments.
//    In the latter case, the function iterates over all key/value pairs.
//    Nesting of objects is allowed.
//    The return values are mapped into a result array.
//
//  supports foo(x, y ,z, key, value)
//  supports foo(x, y, z, { key: value}) -> foo(x, y, z, key, value)
//  supports foo(x, y, {z: { key: value} }) -> foo(x, y, z, {key: value}) -> foo(x, y, z, key, value) 
function unfoldArgs( foo ){

    //unfold the rest object; recursive.
    function unfold(nest, args, rest){

        if( (nest > 0)  && ($.type(rest) === "object") ){   

            return Object.keys(rest).map( function(key){
                return unfold(nest - 1, args.concat(key), rest[key] );
            });

       } else {

            if(rest !== undefined) args.push(rest);
            return foo.apply(this, args );

        }
    }

    return function( /*args*/ ){

        var args = Array.from( arguments ), //[].slice.apply(arguments),
             //foo.length = number of declared arguments on foo
            nest = foo.length - args.length,  //nesting level
            rest = nest > 0 ? args.pop() : undefined;  

        return unfold( nest, args, rest );
    }
}

Example for use in Bliss:

//object = $.lazy(object, property, getter)
//object = $.lazy(object, properties)
var $.lazy = unfoldArgs( function(object, property, getter){ ... });

//subject = subject._.delegate(type, selector, callback)
//subject = subject._.delegate(type, selectorsToCallbacks)
//subject = subject._.delegate(typesToSelectorsToCallbacks)
var $.delegate = unfoldArgs( function(type, selector, callback){ ... });

//$.live = function(object, property, descriptor)
//$.live = function(object, properties)
var $.live = unfoldArgs( function(object,properties){ ... });

//subject = subject._.set(property, value)
//subject = subject._.set(options)
var $.set = unfoldArgs( function(property, value){ ... });

//subject = subject._.once(type, callback)
//subject = subject._.once(typeToCallbacks)
var $.once = unfoldArgs( function(type, callback){ ... });

//!!NOK, cause the variable argument is not the last parameter !
//$.add = function (name, callback [, on])
//$.add = function (callbacks [, on])
//$.add = function (methods, on, noOverwrite)
var $.add = ???

Examples how this could be used / tested :

function test(a,b,c, key, value){
    console.log( "a:"+a,"b:"+b,"c:"+c,"key:"+key,"value:"+value, arguments)
}
var test1 = unfoldArgs( test );

console.log("****test1");
test1( 3, 4, 5, "key", "value");  //invokes test() once
test1( 3, 4, 5, { key1: "value1", key2: "value2" }); //invokes test() twice

console.log("****test2");
test1( 3, 4, "type", "key", "value");   //invokes test() once
test1( 3, 4, "type", {key1: "value1", key2: "value2"});   //invokes test() twice
test1( 3, 4, {type1: {key1: "value1", key2: "value2"}, 
              type2: {key1: "value1", key2: "value2"}});   //invokes test() four times

console.log("****error cases")
test1( 3, 4, "type", "xyz");  //invokes test() once with 4 parameters
test1( 3, 5, "type", 1234567); //invokes test() once with 4 parameters
test1( 3, 4, "type", true); //invokes test() once with 4 parameters
test1( 3, 4, "type", [1,2,3]);  //invokes test() once with 4 parameters 
test1( 3, 4, "type", function(a){ return true;} );  //invokes test() once with 4 parameters
test1( 3, 4, "type", "key", "value", "too much");  //invokes test() once with 5 parameters

@dperrymorrow
Copy link
Collaborator

I am finishing tests for $.live now and then i feel we have the test coverage needed to tackle this.
As long as all the tests all check for the different parameter situations like this test.
https://github.com/LeaVerou/bliss/blob/gh-pages/tests/objects/LazySpec.js#L86-L101
thanks for all the input @brushed , much appreciated.

@dperrymorrow dperrymorrow self-assigned this Dec 13, 2015
@brushed
Copy link

brushed commented Dec 14, 2015

I made a few improvements to my last post.
Still a few corner-cases to be looked at. ( chainable function or map() )

@LeaVerou
Copy link
Owner Author

Wouldn't it solve the $.add() case and improve performance if we just passed an index and a number of levels (1 by default) to the function?

@dperrymorrow
Copy link
Collaborator

hey all, just finished up tests for $.live i think Im at a good place to tackle this.
Ill submit a pull request soon with the initial attempt at this.

@dperrymorrow
Copy link
Collaborator

working on this now,
possible function names

  • loopArgs
  • unfoldArgs
  • eachArgs
  • ?

@LeaVerou
Copy link
Owner Author

condensedArgs? overload? multiplied? enmasse?

I kinda like overload, though it's overly generic. Doubt we'll ever add anything for more overloading though.

@dperrymorrow
Copy link
Collaborator

cool, I have live working (with tests passing) with the helper method, the implementation looks like this.

live: function(obj, prop, desc) {
  $.overload(arguments, function (property, descriptor) {
    // code for $.live
  });

  return obj;
}

and the helper method looks like thus so far, pretty simple, but may have to add a bit more when i get to some of the more complicated methods. It takes an index param that defaults to 1 like you suggested.

overload: function(args, callback, index) {
  index = index || 1;
  var name = args[index], value = args[index + 1];

  if (!value) {
    for (var key in name) {
      callback(key, name[key]);
    }
  }
  else {
    callback(name, value);
  }
}

any feedback is much appreciated.

@LeaVerou
Copy link
Owner Author

Looks nice so far!!

@LeaVerou
Copy link
Owner Author

although, I think this would fail in cases like $.add() where the collapsible arguments are not last.

@dperrymorrow
Copy link
Collaborator

yeah ill have to sort something out for that to be more flexible.
Ill burn that bridge when i get to it as they say...

@LeaVerou
Copy link
Owner Author

A more flexible check (e.g. is the first argument a string or an object?) would do, I think.

@LeaVerou
Copy link
Owner Author

Also, I was thinking of something that would wrap the function we passed to it. That way, there's no need to pass argument objects around. The wrapper function will call the callback via apply, after adjusting the arguments array on every iteration.

@brushed
Copy link

brushed commented Dec 16, 2015

One more iteration of the unfoldArg() helper function; now also allowing a different position of the foldable argument.

unfoldArg() generates a wrapper function, unfolding any Object argument into key-value pairs. By default the argument to be unfolded is the last argument, you can give a position parameter to overwrite the default. Depending on the signature of the main function, multiple levels of unfolding will happen. (eg. needed for $.delegate() ) The wrapper function is also chainable.

//Helper function to unfold any Object argument into key-value pairs
//Arguments:
//  foo: the function with one unfoldable arg
//  pos: position of the foldable argument counting from the end of the argument list (default=1)
function unfoldArg(foo, pos){

    return function( /* args */ ){

        var args = Array.from(arguments), //[].slice.apply(arguments),
            nest = foo.length - args.length,  //foo.length = number of declared arguments on foo
            arg =  args.length - ( pos === undefined ?  1 : pos ),
            head = args.slice(0, arg),
            tail = args.slice(arg + 1);

        ( function unfold(nest, arg, keys){

            if( (nest > 0)  && ($.type(arg) === "object") ){

                for( var key in arg ){

                    unfold( nest-1, arg[key], keys.concat(key) );

                }

            } else {

                foo.apply(this, head.concat(keys, arg, tail) );

            }
        } )( nest, args[arg], [] );

        return this; //make chainable
    }
}

Usage :

//object = $.lazy(object, property, getter)
//object = $.lazy(object, properties)
var $.lazy = unfoldArg( function(object, property, getter){ ... });

//subject = subject._.delegate(type, selector, callback)
//subject = subject._.delegate(type, selectorsToCallbacks)
//subject = subject._.delegate(typesToSelectorsToCallbacks)
var $.delegate = unfoldArg( function(type, selector, callback){ ... });

//$.add = function (name, callback, on, noOverwrite)
//$.add = function (callbacks, on, noOverwrite)
var $.add = unfoldArg(function(name,callback,on,noOverwrite){ ...}, 3);

@dperrymorrow
Copy link
Collaborator

sure thing, ill wrap it,
I now have tests passing on $.lazy, $.live, and $.set

@brushed thanks

@LeaVerou
Copy link
Owner Author

@brushed I like how this does everything automatically, but:

  1. We don't want to unfold any object argument. There are tons of functions that accept objects as arguments with various options, and we don't want those unfolded.
  2. To manage to do everything automatically, it ends up requiring much more code than @dperrymorrow’s solution and the recursiveness, while elegant, also makes it confusing.

I think providing an index and depth, with reasonable defaults, is the more understandable and simpler solution, even if less elegant.

@brushed
Copy link

brushed commented Dec 16, 2015

unfoldArg(..) does not unfold any object argument, only the one indexed with the 'pos' argument, or, default, the last argument.
While you could add a depth parameter (default 1) this can easily deduced from the argument list of foo; comparing it to the actual nummber of passed arguments.

@brushed
Copy link

brushed commented Dec 16, 2015

The complexity really comes from the fact we'd like to support multiple depths. If we'd limit that (practically probably only 2 levels would ever be used) we could avoid the recursion and still keep an elegant solution.

@LeaVerou
Copy link
Owner Author

Sorry for missing pos, I didn't have time to understand the code in detail, I just skimmed over it.
I’m a bit skeptical about automatically determining the number of levels. Just because the values are objects does not necessarily mean they should be unfolded, and there is currently no way to protect against that. Using the argument list of foo is also a bit flimsy: What if foo has other optional arguments, which are handled internally by the passed function?

I would much rather provide the number of maximum levels (defaults to 1) and keep things simple & predictable. I love heuristics too, but they can be dangerous when there’s no way out.

Btw, to be clear, big thumbs up for this code. It’s very clever and elegant, even if not 100% practical in this case!

@brushed
Copy link

brushed commented Dec 17, 2015

Just to be clear, the number of levels is detected based on the number of arguments in the signature of foo versus the number of arguments passed when calling the wrapped function.

But indeed this means that optional arguments (eg such as in $.add()) are not ok.

@LeaVerou
Copy link
Owner Author

Exactly. Like I said, I love heuristics too, but good heuristics fulfill two conditions:

  1. The number of use cases where they fail is tiny
  2. Opt-out is possible for those use cases.

@LeaVerou
Copy link
Owner Author

Since we now have $.overload(), I’m closing this.
We do need docs for it though...

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

3 participants