Cannot return from async method

Question

I am trying to define a class for entities that are drawn into an html canvas. The class gets defined as:

//classes for smallworld

var Smwo = function (num1,num2,num3,bool1,num4,imageL)
{
  this.posX = num1,
  this.posY = num2,
  this.margin = num3,
  this.blocking = bool1,
  this.intercept = num4,
  this.imageLink = imageL,
  this.spawn = function(xer,yer)
  {
    var randX = Math.floor(Math.random() * xer);
    var randY = Math.floor(Math.random() * yer);
    this.posX = randX;
    this.posY = randY;
    this.loadedImage = new Image();
    this.loadedImage.onload = function()
    {
      //register as live actor
      console.log("Loaded");
      return this.loadedImage;
      console.log(this.loadedImage);
    }
    this.loadedImage.src = this.imageLink;
  },
  this.act = function()
  {
    //unit behaviours
  },
  this.tick = function()
  {
    //unit lifecycle
  }
};

Back in my window.onload that fires everything off I then try:

//spawn test
smallWorld.newTree = new Smwo(10,10,10,true,5,"tree.png");
var drawImage  = smallWorld.newTree.spawn(cWi,cHi);
console.log(smallWorld.newTree);
console.log(drawImage);
context.drawImage(drawImage,10,10);

smallWorld is my global namespace, Ive also tried removing this namespace and get the same result. My console output shows I'm building a new instance of Smwo but drawImage is supposedly undefined and therefore the context.drawImage fails because the first passed variable isn't an image type.

What am I missing here? I have an explicit return of the smwo.loadedImage, is that somehow not enough?

return this.loadedImage; is return to this.loadedImage.onload not spawn, and this is the function assigned to this.loadedImage.onload, not your object — Josh Lin, Aug 09 '16 at 08:34
@Pavlo: No - the code needs to be [*working*](https://codereview.stackexchange.com/help/on-topic) for CodeReview. — Amadan, Aug 09 '16 at 08:40
@Amadan you're right. The question confused me: I thought it was about applying OOP principles in JS. — Pavlo, Aug 09 '16 at 08:42
look into prototypes. assigning functions to `this` in the constructor is inefficient as it will create a new function for every invocation of the constructor. — Dan, Aug 09 '16 at 08:45
@LittleAlien. Canvas is important because it relies on asynchronous resource loading. Look into HTML canvas for more info. — KolKurtz, Aug 09 '16 at 08:55
@Little Alien, sometimes the spawn method wont be called but the x and y will still need to initialised. Hope that helps you understand. — KolKurtz, Aug 09 '16 at 08:59
You are saying that using intermediate variables helps to initialize `this.variable`? Yes, assigning a value to `this.variable` right away is terrible and does not initialize the field. Thank you for making the sense. — Little Alien, Aug 09 '16 at 09:03
@Little Alien, the intermediates for the rands are because the rands will be re-used later. Does this relate to a correction that you have figured out? — KolKurtz, Aug 09 '16 at 09:06
Yes, I see. The parts of your code are not seen because `you have posted the full code`, as required by crazy rules. That is obvious and makes immense sense. I do not feel like I am losing the ground under my feet. I have lost it many years ago. — Little Alien, Aug 09 '16 at 09:08

score 4 · Accepted Answer · edited May 23 '17 at 12:14

spawn doesn't have a return, thus gives back undefined, which you assign to drawImage. You can't return from an asynchronously invoked function - return inside this.loadedImage.onload handler will not do what you want.

You either want to go with promises or callbacks. This is the simplest fix:

this.spawn = function(xer,yer,callback)
  {
    var randX = Math.floor(Math.random() * xer);
    var randY = Math.floor(Math.random() * yer);
    this.posX = randX;
    this.posY = randY;
    this.loadedImage = new Image();
    this.loadedImage.onload = function() // `this` here is `this.loadedImage`
    {
      //register as live actor
      console.log("Loaded");
      callback(this);
    }
    this.loadedImage.src = this.imageLink;
  }

and then

var drawImage  = smallWorld.newTree.spawn(cWi,cHi, function(drawImage) {
    console.log(smallWorld.newTree);
    console.log(drawImage);
    context.drawImage(drawImage,10,10);
});

Nothing to do with object orientation, and everything to do with the fact that "How do I return the response from an asynchronous call?" does not have an answer.

EDIT to your edit:

I have an explicit return of the smwo.loadedImage, is that somehow not enough?

No. spawn will assign that function to a handler then return undefined. Browser will some time later trigger the load event, invoke the handler function, then you return drawImage to the browser, which will discard it without effect (if it was a click handler, for example, the browser would use this return value to decide whether to consider the click event handled or not; with load event it is irrelevant). But that value never reaches your code.

EDIT because silly to forget the this context switch

Perfect answer, thanks very much. I should be using more callbacks, I get it. I'm just not used to the timing in javascript still. — KolKurtz, Aug 09 '16 at 08:38
EDIT: although it still doesnt work. The console.log(drawImage) still comes back as undefined and the context draw still fails. — KolKurtz, Aug 09 '16 at 08:48
Ah... `this` might be the problem; inside the handler, `this` will be the loaded image, so `this.loadedImage` would be the non-existent loaded image attribute of the loaded image. Try just `callback(this)` instead of `callback(this.loadedImage)`. Or use the `that = this` trick, or `bind` to make sure you know what `this` is. EDIT: bleh, my brain doesn't work, so many corrections :P — Amadan, Aug 09 '16 at 08:56
Well done Amadan! The extra .loadedImage was unneccessary you were right. That is already what you were referring to. Clearly not an OO problem in the end. Apologies for my confusion. Entities now loading as expected. — KolKurtz, Aug 09 '16 at 09:04

Cannot return from async method

1 Answers1