cagataycali / lazy

Lazy, AI chatbot service.
https://cagatay.me
MIT License
160 stars 18 forks source link

Promisify nedb & request #8

Open anonrig opened 7 years ago

anonrig commented 7 years ago

Because of the lack of bluebird in this library, you had to promisify nedb from scratch. But basically, you could have used Promise.promisifyAll(Datastore.prototype);.

On the other hand, request-promise library already promisifies request library.

cagataycali commented 7 years ago

First of all thank you very much for your contribution, It made me happy to share your information with me.

On the other hand, I did not do that because the second solution will add extra dependency. request-promise

If you have any other ideas, I would like to talk and discuss fondly.

Thank you again for your contribution. Çağatay

anonrig commented 7 years ago

It's proven in the past that Bluebird's (https://github.com/petkaantonov/bluebird) Promise implementation is faster than the actual Promise in NodeJs.

Following code will promisify all functions that include callbacks. Therefore you don't need to do return new Promise((res, rej) every time.

const Promise = require('bluebird');
Promise.promisifyAll(Datastore.prototype);

For example: you can just use the following code:

return this.categories.findOne({name: obj.category});

On the other hand using Bluebird will enable this function:

quiet() {
    return new Promise(resolve => {
      this.slient = !this.slient;
      resolve(slient);
    });
  }

to be replaced by this:

quiet() {
  this.slient = !this.slient;
  return Promise.resolve(this.slient);
}

On the other hand, when promisifying an actual class/function that you didn't write from scratch, developers tend to make mistakes. Therefore, adding request-promise will have more benefits for the library in the long term.

But again, it's your choice. Best.

cagataycali commented 7 years ago

I'll rewrite lazy with your instructions in few days Thanks your advices 👍