CODEWITHSUNDEEP
javascriptnode.jstypescriptes6-promise
user1790300
user1790300

Reputation: 1735

Node.js: Returning original Promise vs. a new Promise instance

I am trying to fix some code that is really Promise heavy I am trying to fix is by removing the new Promise declaration and returning the initial Promise instead, but am trying to make sure that I am properly tanslating the before and after. Below I have an example outlining one of the scenarios that need to be changed. Am I missing something between the before and after code or am I traveling in the right direction? Also,for the after, will I be able to catch all of the rejected errors?

Before:

    add(book: IBook, authorId: string): Promise<IBookModel> {
        let p = new Promise((reject, resolve) => {
            let newBook: IBook = new Book({…book});
            newBook.createInitialSetup();
            let bookReady = {... newBook};
            this.bookRepository.add(bookReady)
                .then((completedBook: IBookModel) => {
                    this.bookRepository.findAuthorById(authorId)
                        .then((author: IAuthorModel) => {
                            let newBookAuthor: IAuthorModel = new Author();
                            newBookAuthor(completedBook._id, author._id);

                            let finalBookAuthor = {... newBookAuthor} as IAuthor;

                            this.bookRepository.addBookAuthor(finalBookAuthor)
                                .then((result: IBookAuthorModel) => {
                                    resolve(completedBook);
                                })
                                .catch((err: Error) => {
                                    reject(err);
                                });
                        })
                        .catch((err: Error) => {
                            reject(err);
                        });
                })
                .catch((err: Error) => {
                    reject(err);
                });
        });

        return p;
    }

After:

    add(book: IBook, authorId: string): Promise<IBookModel> {
         let groupSetup: IGroupModel = new Group({...group});
         newBook.createInitialSetup();
         let bookReady = {... newBook};

         return this.bookRepository.add(bookReady)
                    .then((completedBook: IBookModel) => {
                        return this.bookRepository.findAuthorById(authorId)
                                   .then((author: IAuthorModel) => {
                                        let newBookAuthor: IAuthorModel = new Author();
                                         newBookAuthor(completedBook._id, author._id);

                                        let finalBookAuthor = {... newBookAuthor} as IAuthor;

                                        return this.bookRepository.addBookAuthor(finalBookAuthor)
                                                    .then((result: IBookAuthorModel) => {
                                                           return completedBook;
                                         });                  
                        });
         });
    }

Upvotes: 0

Views: 68

Answers (2)

Titian Cernicova-Dragomir
Titian Cernicova-Dragomir

Reputation: 249536

Async/await might make your code easier to understand and read:

async add(book: IBook, authorId: string): Promise<IBookModel> {
    let newBook: IBook = new Book({ ...book });
    newBook.createInitialSetup();
    let bookReady = { ...newBook };
    let completedBook: IBookModel = await this.bookRepository.add(bookReady)
    let author: IAuthorModel = await this.bookRepository.findAuthorById(memberId)

    let newBookAuthor: IAuthorModel = new Author();
    newBookAuthor(completedBook._id, author._id);

    let finalBookAuthor = { ...newBookAuthor } as IAuthor;

    let result: IBookAuthorModel = await this.bookRepository.addBookAuthor(finalBookAuthor);
    return completedBook
}

Just a couple of notes:

  1. Not sure who memberId is it is not defined in the original code
  2. You could surround with try/catch the whole code but since all you do is forward the error, this is not needed as the returned Promise will be rejected if any of the awaited promises will fail
  3. Take it with a grain of salt, the general principle of using async/await is valid, but the conversion was performed under a screaming baby background so it might have errors.

Upvotes: 3

basarat
basarat

Reputation: 275799

Am I missing something between the before and after code or am I traveling in the right direction?

Traveling in the right direction.

More

Also don't nest then calls. Chain then: 

add(book: IBook, authorId: string): Promise<IBookModel> {
     let groupSetup: IGroupModel = new Group({...group});
     newBook.createInitialSetup();
     let bookReady = {... newBook};

     return this.bookRepository
                .add(bookReady)
                .then((completedBook: IBookModel) => {
                    return this.bookRepository.findAuthorById(memberId)
                })
                .then(///so on

Upvotes: 3

Related Questions