2

I'm wondering what route I should take to make this code work "as intended." The API call is asynchronous -- so the constructor returns before data is loaded.

addSongById: function (songId) {
    var song = new Song(songId);
    console.log(song);
    this.addSong(song);

    if (this.songCount() == 1)
        this.play();

    UserInterface.refresh();
    SongGrid.reload();
},

function Song(songId) {
    $.getJSON('http://gdata.youtube.com/feeds/api/videos/' + songId + '?v=2&alt=json-in-script&callback=?', function (data) {
        this.id = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8); return v.toString(16); });
        this.songId = songId;
        this.url = "http://youtu.be/" + songId;
        this.name = data.entry.title.$t;
    });
}

Is it possible to force the constructor to not return prematurely? Ideally I wouldn't have to pass an arbitrary amount of parameters into the Song constructor and bring information only relevant to Song outside the scope of it..

Sean Anderson
  • 27,963
  • 30
  • 126
  • 237

4 Answers4

7

As with most asynchronous operations, I'd use a Deferred in this situation; constructors in JS are not obliged to return an instance of themselves:

function Song(songId) {
    var song = this;
    var def = new $.Deferred();
    $.getJSON('http://gdata.youtube.com/feeds/api/videos/' + songId + '?v=2&alt=json-in-script&callback=?', function (data) {
        song.id = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8); return v.toString(16); });
        song.songId = songId;
        song.url = "http://youtu.be/" + songId;
        song.name = data.entry.title.$t;
        def.resolve(song);
    });
    return def.promise();
}

var promise = new Song(songId);
promise.done(function(instance) {
    // you've got a Song instance
});
lanzz
  • 42,060
  • 10
  • 89
  • 98
  • Way nicer, than the callback solution. – Jonathan Gruber Apr 08 '15 at 09:08
  • 1
    very old thread I know, but rather then returning the promise, why not to store it in some `this.promise = def.promise();` then on the async callback `def.resolve(this)`? that way the constructor returns the object, and the callback can be chained this way: `var song=new Song(songId); song.promise.done(function(){doWhatever()})` – undefinederror May 11 '15 at 09:49
  • @undefinederror makes a very good point. A singleton constructor should never return anything but its own instance – Christophe Marois Apr 21 '17 at 17:41
3

You may want to use $.ajax instead of $.get with option async: false. However this will lock execution of every other javascript. And this may be a problem if for example the server won't respond for any reason.

Therefore this is a bad practice. Use callbacks, for example

function Song(songId, callback) {
    var self = this;
    $.getJSON('http://gdata.youtube.com/feeds/api/videos/' + songId + '?v=2&alt=json-in-script&callback=?', function (data) {
        self.id = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8); return v.toString(16); });
        self.songId = songId;
        self.url = "http://youtu.be/" + songId;
        self.name = data.entry.title.$t;
        callback();
    });
}

var song = new Song(songId, function() {
    // The other code goes here.
});
freakish
  • 54,167
  • 9
  • 132
  • 169
3

...it is a bad practice. A constructor should return an instance of its class, nothing else. It would mess up the new operator and inheritance otherwise.

Moreover, a constructor should only create and initialize a new instance. It should set up data structures and all instance-specific properties, but not execute any tasks. It should be a pure function without side effects if possible, with all the benefits that has.

Ref: Is it bad practice to have a constructor function return a Promise?

Community
  • 1
  • 1
ignoramous
  • 89
  • 1
  • 8
0

sync call may cause browser to stop responding and some browser may kill you script forcefully.

Jquery has an option while making a request to a URI

asyncBoolean Default: true

By default, all requests are sent asynchronously (i.e. this is set to true by default). If you need synchronous requests, set this option to false. Cross-domain requests and dataType: "jsonp" requests do not support synchronous operation. Note that synchronous requests may temporarily lock the browser, disabling any actions while the request is active. As of jQuery 1.8, the use of async: false is deprecated.

http://api.jquery.com/jQuery.ajax/

Hope this will be helpful

Regards.

Shoaib Shaikh
  • 4,565
  • 1
  • 27
  • 35