JQuery Promise Interface for a (very) simple JSON Parser

Question

I have written a parser function that returns a JQuery Promise. You can see from the code that this is the top level parser and it delegates out to two other more specific parsers.

At the minute it feels a little scattered about with the promise being rejected and resolved all over the place. perhaps it would be better with a try and catch and an error handler that rejected the promsie? how would others approach this?

/**
 * Inspects JSON and delegates to either the sourceParser or the dataParser.
 * 
 * @constructor 
 * @param {Object} json
 * 
 * @returns JQuery Promsise
 * done: -
 * fail: error message
 * progress: Progress message
 */
function Parser(json)
{
 var dfd = $.Deferred();
 dfd.notify("Parsing");
 if (json && typeof json === "object")
 { 
 if (json.hasOwnProperty("GetDataSources"))
 { 
 var dataSource = json.GetDataSources;
 if (this.isSuccessfulResponse(dataSource.Response))
 {
 // Notify the caller of any progress
 dfd.notify("Parsing Source");
 // Create a new Source Parser
 var sourceParser = new SourceParser(dataSource);
 sourceParser.done(dfd.resolve);
 sourceParser.fail(dfd.reject);
 }
 else
 {
 dfd.reject("Parsing Source Failed");
 }
 }
 else if (json.hasOwnProperty("GetData"))
 { 
 var data = json.GetData;
 if (this.isSuccessfulResponse(data.Response))
 {
 // Notify the caller of any progress
 dfd.notify("Parsing Data");
 // Create a new Data Parser
 var dataParser = new DataParser(data);
 dataParser.done(dfd.resolve);
 dataParser.fail(dfd.reject);
 }
 else
 {
 // Pass back an error message?
 dfd.reject("Parsing Data Failed");
 }
 }
 }
 else
 {
 dfd.reject("There was a problem reading the JSON")
 }
 return dfd.promise();
}

Answer 1

Chop up functions longer than 12 lines of code into smaller functions. Since you're using this, then you can attach the additional functions to the prototype on Parser.

Here's what I came up with.

function Parser(json) {
 var dfd = $.Deferred();
 dfd.notify("Parsing");
 if (json && typeof json === "object") {
 if (json.hasOwnProperty("GetDataSources")) {
 this._handleData( dfd, json.GetDataSources, SourceParser, "Source" );
 } else if (json.hasOwnProperty("GetData")) {
 this._handleData( dfd, json.GetData, DataParser, "Data" );
 }
 } else {
 dfd.reject("There was a problem reading the JSON");
 }
 return dfd.promise();
}
Parser.prototype._handleData = function(dfd, data, OtherParser, type){
 if (data && this.isSuccessfulResponse(data.Response)) {
 dfd.notify("Parsing " + type );
 var otherParser = new OtherParser(data);
 otherParser.done(dfd.resolve);
 otherParser.fail(dfd.reject);
 } else {
 dfd.reject("Parsing "+ type +" Failed");
 }
};

• JonWells: Thanks Larry. I had improved the code since posting it here but your approach is even better still. I have a tendency to create promises on top of promises unnecessarily! and I liked passing the parser as an Argument.
• JonWells: one thing though sinsided the hasOwnProperty functions should they be something like dfd = this._handleData otherwise the dfd object gets lost.
• Larry Battle: It's not really needed because objects are passed by reference and primitive values are passed by value in javascript. But I like returning the value of dfd to check the output since dfd is a private member. However, if return dfd; causes confusion then take it out.
• JonWells: have I mentioned that this is awesome?
• JonWells: Out of interest, Parser is a constructor function, do you see anything wrong with it returning a promise rather than an instance of itself?