6
\$\begingroup\$

As a beginner I am always determined to improve myself. I've got written the following Code using jQuery that is working fine. However, I am sure that there are cleaner ways for achieving the same.

Right now, I am struggling with the this keyword and the asynchronous loading of the JSON file. Moreover, I am not sure whether you should call an function for initialization the way I did.

Do you have any suggestions for improvements?

$(function(){ function Model() { this.data = null; this.init(); }; Model.prototype = { deferred: $.Deferred(), config: { jsonFile: 'dump.json' }, init: function() { this.loadJson(); }, loadJson: function() { var self = this; jQuery.getJSON( this.config.jsonFile, function(data) { self.data = data; self.deferred.resolve(); } ) .fail(function() { console.error("Loading JSON file failed"); }) .always(function() {}); }, getData: function(callback) { console.log("getData") var self = this; $.when(this.deferred) .done(function() { callback(self.data) }) }, }; var model = new Model(); model.getData(function(data) { console.log(data) }); }); 

(duplicate of https://stackoverflow.com/q/23142089/3546614)

Update 1

I've just updated my code (and truncated unimportant stuff) taking @jgillich's advices into account. For the moment I feel better reading the JSON file when the object is generated which is why I outsourced the operation into separate function.

$(function(){ function Model() { this.loadJson(); }; Model.prototype = { deferred: $.Deferred(), jsonFile: 'dump.json', loadJson: function() { $.getJSON(this.jsonFile) .done(function (data) { this.data = data; this.deferred.resolve(data); }.bind(this)) .fail(this.deferred.reject); }, getData: function() { return this.deferred.promise(); }, }; var model = new Model(); model.getData().done(function(data) { console.log('done') console.log(data) }); }); 
\$\endgroup\$

    1 Answer 1

    3
    \$\begingroup\$
    • Storing deferred in the model and then passing a callback to getData is a bit obscure. You could write it like this instead:

      getData: function() { var deferred = $.Deferred(); if(!this.data) { $.getJSON('...',).done(function (data) { this.data = data; deferred.resolve(data); }.bind(this)).fail(deferred.reject); } else { deferred.resolve(this.data); } return deferred.promise(); } model.getData().done(function (data) { /* ... */}); 
    • Use either $ or jQuery, don't mix them.

    • Use Function.prototype.bind instead of var self = this.

    • Use semicolons after each statement (or leave them out entirely if you prefer that).

    • this.data = null; seems pointless, newly created objects don't have such a property.

    \$\endgroup\$
    0

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.