search

bdswiss / country-language

Node.js module for i18n apps - query any country's spoken languages or find countries where a language is spoken.
MIT License
80 stars 29 forks source link

Support for Promises #6

Open zeke opened 6 years ago

zeke commented 6 years ago

Hi @TassosD and @pauliusuza. This is a great library!

I'd be willing to open a PR to add support for Promises.

Would that be a welcome contribution?

boenrobot commented 4 years ago

Is that really needed though? As it stands, the callbacks, and the whole code in this package, is executed synchronously. There are no Node async calls or 3rd party promise based code triggered anywhere. Functions that accept callback also return the result or an error message as a string.

To be honest, I'm wondering more why are there even callbacks in the first place, rather than exceptions being thrown on errors, and just the result in normal circumstances.

zeke commented 4 years ago

Hey @boenrobot I hadn't actually looked into the implementation. If there's a way to make this lib synchronous (instead of promisifying it), that would also be an improvement.

boenrobot commented 4 years ago

It already is, that's my point.

As the read me says near the bottom, the callback arguments are optional, and if not provided, either the result will be returned, or a string that is the error message.

Even if passed though, they are still executed on the same tick. If you dig through the source, you'll notice that the return value of the callback is actually the return value of the function accepting it, and there's a default callback that just returns the error or the result.

I hate callbacks and prefer promises too, but for this lib, since it's sync anyway, it's better to just not use callbacks.