Split an object into arrays

Question

I'm currently writing a library using jQuery, and I'm facing a wall I can't pass alone.

First, here is some code :

(function($) {
// default options
var defaults = {
    option1: "what ever",
    option2: "what ever"
};

// available methods
var methods = {
    method1: function(param1) {
        console.log(param1);
        /* ... */
    },
    method2: function(param1, param2) {
        console.log(param1);
        console.log(param2); // gives undefined
        /* ... */
    }
};

// Where magic happens
$.fn.pskChart = function(params) {
    if (methods[params] != undefined) {
        if (this.length > 0) {
            return $(this).each(function(i) {
                return methods[params].apply($(this), Array.prototype.slice.call(arguments, 1));
            });
        }
    } else {
        $.error("Method " + params + " doesn't exist for pskChart");
    }
}
})(jQuery);


$("#sample").pskChart("method1", "param1"); // will work
$("#sample").pskChart("method2", "param1", "param2"); // won't work

This code is working in case I provide only two parameters (method and another parameter), but won't work if I have more parameters

I understand that Array.prototype.slice.call(arguments, 1) returns only one object containing all the remaining arguments.

In the second example, method2 will be called with only one parameter that contains ["param1","param2"]

I would like to "split" this object in order to provide as many parameters I have.

I feel like methods[params].apply($(this), Array.prototype.slice.call(arguments, 1)); but I have no clue how to fix it.

Note that I could have an infinite (or at least big) number of parameters (in addition to the method, that will always be first). Playing with Array.slice would not be a great (nor proper) solution.

Side note: jQuery plugins should always return this (barring having a good reason not to). Your pskChart function doesn't. — T.J. Crowder
– T.J. Crowder, Commented Dec 29, 2014 at 13:50
@T.J.Crowder I actually return something in each method (in the /* ... */- which is not relevant for my question) — pistou
– pistou, Commented Dec 29, 2014 at 13:52
Returning things from your methods is pointless, unless you want to return false to stop the each loop. (You should almost certainly remove the return from the line inside the each.) I'm talking about pskChart, not the methods. — T.J. Crowder
– T.J. Crowder, Commented Dec 29, 2014 at 13:55
I should have been more clear: Your pskChart method's return value is wrong in a couple of ways: If pskChart has the method but there are no elements in the jQuery set, it returns undefined; it should return this. If there are elements in the set, it returns a new jQuery object for the set (because you're using $(this) where this is already a jQuery set); it should return this, not a new object. — T.J. Crowder
– T.J. Crowder, Commented Dec 29, 2014 at 13:57

T.J. Crowder · Accepted Answer · 2014-12-29 14:03:25Z

I don't see why it would work even with just one parameter, as you're using the wrong arguments object.

Instead (see comments):

// Where magic happens
$.fn.pskChart = function(params) {
    var args; // **Change** We'll use this below
    if (methods[params] != undefined) {
        if (this.length > 0) {
            // **Change** Grab the arguments here, in the `pskChart` function
            args = Array.prototype.slice.call(arguments, 1);
            return $(this).each(function(i) {
                // **Change** Use them here; you can't use `arguments` here
                // because, it will be the arguments to this anonymouus
                // function, not the ones to `pskChart`
                return methods[params].apply($(this), args);
            });
        }
    } else {
        $.error("Method " + params + " doesn't exist for pskChart");
    }
}

Side note: params is a very strange name for an argument you're using as a method name. Perhaps methodName?

Side note 2: Rather than repeatedly looking up methods[params], consider using a variable. Large sets may benefit from the reduced work.

Side note 3: This line inside your each loop is almost certainly a mistake:

return methods[params].apply($(this), args);

That will return the return value of the method to jQuery's each code. jQuery's each code will completely ignore that vallue unless it's === false, in which case it will stop the each loop. Not at all likely to be what you meant that to do.

Side note 4: Your pskChart function, being a jQuery plugin, should return this unless it has a good reason for returning something else. Right now, the return value is chaotic: If the set has no elements, you return undefined (implicitly); if the set has objects, you return a new jQuery object, not this, because of this line:

return $(this).each(function(i) {

There's no reason for using $() there, this is already a jQuery set (which is why the previous line using this.length works).

Side note 5: You're missing a semicolon from the assignment statement (e.g., there should be one after the closing } on the function expression).

Recommendation:

// Where magic happens
$.fn.pskChart = function(methodName) {
    var args, method = methods[methodName];

    if (!method) {
        // Throws
        $.error("Method " + params + " doesn't exist for pskChart");
    }

    if (this.length > 0) {
        args = Array.prototype.slice.call(arguments, 1);
        this.each(function() {
            method.apply($(this), args);
        });
    }

    return this;
};

Works like a charm, I guess I messed up with some old code I had
@TouPye: Glad that helped. I added a couple of further notes.

Collectives™ on Stack Overflow

Split an object into arrays

1 Answer 1

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Related