JS Get data from a constructor with a Promise

Question

I'm unable to get the data response from unsplashAxios within the GetPhoto constructor, at the moment it just says 'Promise { }'. Have I missed something blatantly obvious or does this need rethinking?

Attempt 1

class Unsplash {
  constructor(path) {
    return new Promise((resolve, reject) => {
      unsplashAxios(path)
      .then(response => {
        resolve(response);
      })
      .catch(error => {
        reject(error);
      });
    });
  }
}

class GetPhoto {
  constructor() {
    console.log('Get Photo');
    const unsplash = new Unsplash('/photos/PgHc0Ka1E0A');
    console.log(unsplash)
    // Console.log Response - Promise { <pending> }
  }
}

Attempt 2

class Unsplash {
  constructor(path) {
    return unsplashAxios(path)
  }
}

class GetPhoto {
  constructor() {
    const unsplash = new Unsplash('/photos/PgHc0Ka1E0A');
    unsplash.then((response) => {
      console.log(response)
    }).catch((response) => {
      console.log(response);
    });
  }
}

Attempt 3 - After @Klaycon I've rewrote the above and this seems to work. But feedback would be great (good or bad).

const unsplashAxios = require('./unsplashAxios');

// Class
class Unsplash {

  // Constructor
  constructor() {
    this.unsplash = null;
  }

  // Method
  getPhoto(id){
    this.unsplash = unsplashAxios( `/photos/${id}`);
    this.unsplash.then((response) => {
      console.log(response)
    }).catch((response) => {
      console.log(response);
    });
  }

  // Method
  getCollection(id){
    this.unsplash = unsplashAxios(`/collections/${id}/photos`);
    this.unsplash.then((response) => {
      console.log(response)
    }).catch((response) => {
      console.log(response);
    });
  }

}

// Instance
myUnsplash = new Unsplash();

// Method
myUnsplash.getPhoto('PgHc0Ka1E0A');

// Method
myUnsplash.getCollection('62890');

Don't return a Promise in a constructor - extract it to an instance method — Ikechukwu Eze, Jan 16 '20 at 22:04
Your `Unsplash` constructor says `return new Promise( ... )`, so `new Unsplash()` is a `Promise`. I don't agree with the pattern you have here though. — Tyler Roper, Jan 16 '20 at 22:06
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! And also [never return a promise from a constructor](https://stackoverflow.com/q/24398699/1048572)! — Bergi, Jan 16 '20 at 22:06
I've created a class as there will be more features to be added e.g getCollection. — Ad., Jan 16 '20 at 22:11

score 0 · Answer 1 · answered Jan 16 '20 at 22:07

You return a Promise. As @Bergi said in your comment, avoid to use them in a constructor.

Use :

unsplash.then((data) => { 
 //do something with data
});

to wait til the promise is resolved. You can read more about promises here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise

score 0 · Answer 2 · answered Jan 16 '20 at 22:12

Since unsplashAxios is already returning a promise, you don't need to wrap it in another promise (you're resolving unsplashAxios's promise then putting that result into another Promise that would have to be resolved elsewhere. Changing your code to this should work:

constructor(path) {
  unsplashAxios(path)
    .then(response => {
      //do something with the response
    })
    .catch(error => {
      //do something with the error
    }
  }
}

score 0 · Answer 3 · answered Jan 16 '20 at 22:46

0

I ended up rewriting and found the following to work as expected:

class Unsplash {
  constructor(path) {
    return unsplashAxios(path)
  }
}

class GetPhoto {
  constructor() {
    const unsplash = new Unsplash('/photos/PgHc0Ka1E0A');
    unsplash.then((response) => {
      console.log(response)
    }).catch((response) => {
      console.log(response);
    });
  }
}

answered Jan 16 '20 at 22:46

Ad.

1
1

`return`ing a value from a class constructor causes the class to be virtually unusable and pointless, as you can't obtain an instance of it in any reasonable manner. this seems like an abuse of es6 classes - just make these normal functions, you don't need to `new GetPhoto()` when `getPhoto()` works exactly the same – Klaycon Jan 16 '20 at 23:11
I’m trying to write the JS to be object orientated and all the tutorials are pointing in this direction. I think the comment ‘Abuse of ES6’ was a little much as I’m knew to writing JS in this style. If you could provide more in-depth knowledge on how to rewrite this feel free to help out. – Ad. Jan 16 '20 at 23:22
classes should be objects which represent a *thing* that can have a *state* and contains functions that modify its state - "GetPhoto" doesn't seem like it's going to have any state, and "Unsplash" currently is just an alternative path to `unsplashAxios()`. I think if you're going to write object oriented javascript you should focus *first* on what objects will have state, and what the state will be - if anything I could see these two class constructors being two methods of some "controller" class. Constructors should only be used to set the initial state, not do other processing, esp. async – Klaycon Jan 16 '20 at 23:30

JS Get data from a constructor with a Promise

3 Answers3