0

I want a class that saves the resolution from an image that has to be loaded first. It will print Width = undefined. The problem is of course that the print function is called before the callback function gets called. After I wait a decent time it will print Width = 20. I am also not sure if its valid to save this to originalThis and pass it to the callback function. How should the code look like so that it works without a random waiting time? I think this is a scenario where you work with Promises?

class MarkerSize {
  markerWidth;
  markerHeight;

  constructor(url) {
    this.getMeta(url, this.saveData);
  }

  getMeta(url, callback) {
    let img = new Image();
    img.src = url;
    let originalThis = this;
    img.onload = function () {
      callback.call(originalThis, this.width, this.height);
    };
  }

  saveData(width, height) {
    this.markerWidth = width;
    this.markerHeight = height;
  }

  printSize() {
    console.log(`Width = ${this.markerWidth}`);
  }
}

let myMarker = new MarkerSize(
  'https://developers.google.com/maps/documentation/javascript/examples/full/images/beachflag.png'
);
myMarker.printSize(); //prints Width = undefined
//Here, I use a timeout function to wait for the image. I know this is really bad style.
setTimeout(function () {
  myMarker.printSize(); //prints Width = 20
}, 3000);
swftowu69
  • 123
  • 1
  • 9
  • Yes, using promises can definitely help with waiting for the right time to print the size, but really [you should not start asynchronous actions (like `.getMeta()`) in a constructor](https://stackoverflow.com/q/24398699/1048572). – Bergi Jul 28 '21 at 22:59
  • Thank you for the information :) – swftowu69 Jul 28 '21 at 23:20

2 Answers2

0

You are right, introducing a Promise will help solve this issue.

printSize method will be asynchronous and will await for loadingPromise.

  async printSize() {
    await this.loadingPromise; // wait for image to be loaded and markerWidth to be set
    console.log(`Width = ${this.markerWidth}`);
  }

loadingPromise should be created in constructor and will resolve when image has loaded.

  constructor(url) {
    this.loadingPromise = new Promise(resolve => {
        this.getMeta(url, (w, h) => {
            this.saveData(w, h);
            resolve();
        });
    });
  }

Full code:

class MarkerSize {
  markerWidth;
  markerHeight;
  loadingPromise;

  constructor(url) {
    this.loadingPromise = new Promise(resolve => {
        this.getMeta(url, (w, h) => {
            this.saveData(w, h);
            resolve();
        });
    });
  }

  getMeta(url, callback) {
    let img = new Image();
    img.src = url;
    let originalThis = this;
    img.onload = function () {
      callback.call(originalThis, this.width, this.height);
    };
  }

  saveData(width, height) {
    this.markerWidth = width;
    this.markerHeight = height;
  }

  async printSize() {
    await this.loadingPromise; // wait for image to be loaded and markerWidth to be set
    console.log(`Width = ${this.markerWidth}`);
  }
}

let myMarker = new MarkerSize(
  'https://developers.google.com/maps/documentation/javascript/examples/full/images/beachflag.png'
);
myMarker.printSize(); //prints 'Width = 20' (asynchonously)

UPDATED Small rewrite as suggested in comments to avoid using callbacks and creating resources in constructor:

class MarkerSize {
  url;
  size;

  constructor(url) {
    this.url = url;
    this.size = null; // not loading image yet, will be loaded during first call to printSize()
  }

  getSize() {
    return new Promise(resolve => {
        let img = new Image();
        img.src = this.url;
        img.onload = function () { // keep in mind that traditional function and arrow function handle 'this' differently
            // here this is bound to Image
            resolve({ width: this.width, height: this.height });
        };
    });
  }

  async printSize() {
    if (!this.size) { // load image if not loaded before
        this.size = await this.getSize();
    }
    console.log(`Width = ${this.size.width}`);
  }
}

let myMarker = new MarkerSize(
  'https://developers.google.com/maps/documentation/javascript/examples/full/images/beachflag.png'
);
myMarker.printSize(); //prints 'Width = 20' (asynchonously)
Vitalii
  • 2,071
  • 1
  • 4
  • 5
  • Better just make `getMeta` return a promise instead of taking a callback. – Bergi Jul 28 '21 at 23:00
  • Thank you for the avesome answer! :) Everything is clear but why is it possible to write `this.getMeta(... =>)` ? How does Javascript check that the function finished loading the image? In the meantime I also solved the Problem and I will post my answer here. Have a nice day! – swftowu69 Jul 28 '21 at 23:14
  • @swftowu69 `getMeta` accepts a callback function and calls it at a later time when image has loaded. In the code above I'm creating a callback function using [arrow function expression](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions) syntax `(w, h) => {...}` – Vitalii Jul 28 '21 at 23:30
  • @Vitalii you are right. I overlooked this. – swftowu69 Jul 28 '21 at 23:34
  • 1
    @Bergi added new version of the code. Keeping the original because it's close to what swftowu69 had in the question – Vitalii Jul 28 '21 at 23:35
0

Here is my Version. What do you think is bad and what can be improved there?

UPDATED VERSION

class MarkerSize {
  #markerWidth;
  #markerHeight;
  #url;

  constructor(url) {
    this.url = url;
  }

  getMeta() {
    return this.#getMetaHelper(this.url)
      .then(img => {
        this.#saveData(img.width, img.height);
        return this;
      })
      .catch(() => {
        const defaultWidthHeight = 10;
        this.#saveData(defaultWidthHeight, defaultWidthHeight);
        return this;
      });
  }

  //load img
  #getMetaHelper(url) {
    return new Promise((resolve, reject) => {
      let img = new Image();
      img.src = url;
      img.addEventListener('load', () => resolve(img));
      img.addEventListener('error', () => reject());
    });
  }

  #saveData(width, height) {
    this.markerWidth = width;
    this.markerHeight = height;
  }

  printSize() {
    console.log(`Width = ${this.markerWidth}\nHeight = ${this.markerHeight}`);
  }
}

let myMarker = new MarkerSize(
  'https://developers.google.com/maps/documentation/javascript/examples/full/images/beachflag.png'
);

myMarker.getMeta().then(obj => {
  obj.printSize();
});
swftowu69
  • 123
  • 1
  • 9
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) in `getMeta`! And don't forget to `reject` your promise when errors occur. – Bergi Jul 28 '21 at 23:20
  • Thanks for the suggestion for improvement but I can't see this antipattern in my code. – swftowu69 Jul 28 '21 at 23:40
  • There's a `new Promise(resolve => {…})` around the `this.#getMetaHelper(this.url).then(…)` call. Just write `getMeta() { return this.#getMetaHelper(this.url).then(img => { this.#saveData(img.width, img.height); return this; }); }` – Bergi Jul 29 '21 at 00:01
  • Thank you @Bergi . I will update my Code and also add error handling. – swftowu69 Jul 29 '21 at 09:22
  • I actually meant `img.addEventListener('error', reject)`, but a timeout isn't bad either :-) – Bergi Jul 29 '21 at 09:27
  • Thanks @Bergi . I will update the code. Can I catch all errors/problems that can occur with the 'error' event? I also found the 'abort' event in the internet. – swftowu69 Jul 29 '21 at 09:48