# how can i make this small jQuery serializeobject util any better

I have this little utility/helper which serializes a form into a javaScript object

            $.fn.serializeObject = function(fn) { var o = {}, a = this.serializeArray(), that;$.each(a, function() {
that = this;
if($.isFunction(fn)){ that = fn(that.name, that.value); } if(o[this.name] !== undefined) { val = that.value || ''; if(!o[that.name].push) { o[that.name] = [o[that.name]]; } o[that.name].push(that.value); } else { o[that.name] = that.value; } }); return o; };  A typical usage of this plugin will be:  someNameSpace = { //... processForm: function (form) { return JSON.stringify(form.serializeObject(function (key, val) { return { name: key, value: encodeURIComponent(val) }; })); } //... };  Any suggestions on how to improve this piece of code? • Maybe improve performance • Maybe its the API-Design • Code readability.. • - ## 1 Answer First of all, I'm barely even sure what's going on here: if(o[this.name] !== undefined) { val = that.value || ''; if(!o[that.name].push) { o[that.name] = [o[that.name]]; } o[that.name].push(that.value); } else { o[that.name] = that.value; }  All the this/that is pretty confusing to look at, especially with all the square brackets mixed in. More descriptive names would help immensely. But also, you check for o[this.name] but then you define o[that.name]. So if my fn function makes all the names identical, it won't work. For instance, if I have 3 inputs (a = 1, b = 2, and c = 3) in a form, and I supply a fn like so: $(form).serializeObject(function (name, value) {
return {
name: "x",
value: value
};
});


I get:

{ "x": 3 }


instead of the expected x: [1, 2, 3]

It'd be much easier to just let me manipulate the objects directly. You wouldn't have any this/that to bother with, and I could simplify my fn to

$(form).serializeObject(function (object) { object.name = "x"; });  No need to build and return a new object. Other things I noticed: • Don't use ... === undefined. Unfortunately, undefined is not a reserved word, meaning that undefined can be defined in some JS runtimes. So if somewhere, some joker has written undefined = 23; it'll break your code. Use typeof x === 'undefined' instead. • Don't use jQuery to do a simple loop. A for loop works quite well, with less overhead and no context-switching. • Don't use jQuery just to check it something's function. Use typeof x === 'function' instead. • Don't use x.push to check if something's an array. Use x instanceof Array. For all you know, x.push could mean anything. (User winner-joiner adds: watch out for frame/iframe situations where instanceof doesn't work) • You have a val variable which is 1) not used, 2) not declared (thus implicitly global!), and 3) if it were used, it'd mess up. I my fn returns a value of, say, false on purpose, it'd become an empty string, because false || '' => ''. Don't do anything clever with the values; especially not after the user's fn may have changed them. Assume the values are correct, regardless of what they are. And again: Use descriptive variable names. It took me quite a while to figure out your code. I'd write it like this: $.fn.serializeObject = function (decorator) {
var source = this.serializeArray(),
serialized = {},
i, l, object;

for( i = 0, l = source.length ; i < l ; i++ ) {
object = source[i];

if( typeof decorator === 'function' ) {
decorator(object);
}

if( serialized.hasOwnProperty(object.name) ) {
if( !(serialized[object.name] instanceof Array) ) {
serialized[object.name] = [ serialized[object.name] ];
}
serialized[object.name].push(object.value);
} else {
serialized[object.name] = object.value;
}
}
return serialized;
}

-
+1 great and complete answer, i learned a lot Thx (would vote even more if possible) –  winner_joiner Mar 6 '13 at 7:41
Thanks so much! really helpful! I will update the gist according to your recommendations and would you mind to please take a second look at it? –  adardesign Mar 6 '13 at 14:00