Object serializer

Question

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..

See original gist

Answer 1

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;
}

• adardesign: 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?