Support formatting based on locale code

[edorivai]

Additionally this commit

• replaces the currencies array with an object, for faster lookup
• adds a couple of localized euro formats,
• uses the locale-currency package to link locale codes to currencies

Please note: I replaced the currencies array with a JSON object. This ensures we don't have to loop over the array to find a matching currency. However, it conflicts with existing functionality of exporting currencies as an array. That's why I've flagged these two exporting tests as pending.

I think we have a couple of options:

• Revert to using an array.
• Publish as a major semver bump, and expose currencies as an object
• Programatically convert the object to an array (with something similar to Object.values())

Personally I think the benefits of using an object are worth the breaking change. We could even take it a bit further, and refactor the formatMapping a bit, so we would be able to completely remove the Array.find polyfill.

[smirzaei]

Looks good overall, but I don't like the breaking change, I know some projects depend on that to just load the currency data and use it in a different way.

Exposing it as an object would make it faster here but pretty awkward to use the currency data somewhere else.

Also is Object.assign ES6 only?

[smirzaei]

I have an idea, what if we keep this structure but still expose currencies in a different way, Like having a function which iterates over the object and returns the data as an array? It would still be a breaking change but would make it easier for people to migrate if they wanted.

Something like

Object.keys(currencies).map(k => currencies[k])

I haven't tested that function so not sure if it works, it's been a while since I've done JS :p

[edorivai]

You mean something like the third option I named? 😛

Programatically convert the object to an array (with something similar to Object.values())

So yeah, Object.values would have been ideal here, but is poorly supported.

Check my latest commit, it is now backwards compatible, but I'm not sure how browser support checks out.

I've added currencies.js, which converts the currencies object to an array the moment it's required. And I've converted index.currencies to a getter, which should convert to an array when it is accessed.

This way we're not performing the computation over all currencies when the client really only needs one; which is the main use case, I suppose.

Let me know if you like how it looks now, I can polish and look at browser support later.

[smirzaei]

You mean something like the third option I named?

Haha yeah, getting sleepy already :p

It looks good to me, just not sure if that getter is supperted. Let me know when you're done and I'll merged it. Then we can decide if if requires a major version bump or not.

Thanks for the improvement :)

[edorivai]

Exposing it as an object would make it faster here but pretty awkward to use the currency data somewhere else.

Honest question: in what real-world use case is it more awkward to receive an object of currencies instead of an array? I'm guessing the main use case is to render prices in a single currency to a single user. For example, I intend to use this lib like so:

const formatCurrency = require('currency-formatter').format;
const locale = determineLocale(); // Locale determining magic
function format(amount) {
    formatCurrency(amount, { localo: locale });
}

Furthermore, if in some case one would really prefer an array, it is easily converted with Object.keys or Object.values if you have babel :)

Let me know when you're done

Yeah, I'll finalize tomorrow, and inform you about compatibility. If you're really keen on backwards compatibility, we could upgrade in two steps;

1. Add locale functionality with currencies array -> minor bump
2. Replace array structure for object structure -> major bump

[smirzaei]

Honest question: in what real-world use case is it more awkward to receive an object of currencies instead of an array? I'm guessing the main use case is to render prices in a single currency to a single user. For example, I intend to use this lib like so:

Well I guess it depends on how you look at it. If you look at it as a list or a hashmap. Imagine if you had to load this data from a REST API... It would look something like this to me

GET /currencies would return a list of currencies.
GET /currencies/USD would return the USD object.

That's why I think returning a hashmap would be a bit awkward, but to be honest don't care that much about it, backwards and ECMAScript 5 compatibility is way more important. If we're not gaining a huge advantage we shouldn't break them.

[edorivai]

Yeah, I understand that when someone expects a list of currencies, an array makes more sense. My point is more that in the context of formatting currencies, I find it hard to imagine real use cases of retrieving the entire list, versus requesting a formatted string for a specific currency. But I'd be happy to be proven wrong 😄

The latest changes retain backwards compatibility by converting to an array for the following cases:

require('currency-formatter').currencies // Array
require('currency-formatter/currencies') // Array

If I'm right, the conversion is not executed unless you access either of these two.

Additionally, you can access the currencies in object notation like this:

require('currency-formatter/currencies.json') // { USD: { ... }, EUR: { ... }, ...more }
require('currency-formatter/currencies.json').USD // { code: 'USD', symbol: '$', ...more }

This has the benefit of O(1) complexity for retrieving specific currencies.

As far as I can see, it is all ES5-compatible. Sadly, Object.assign is not supported by IE11, so the object-assign import remains.

Also, since I replaced the currencies array with an object, the Array.find polyfill was only used for picking the formatting from formatMapping. I felt like having an Array.find polyfill for a 4-length array was a bit overkill, so I removed the polyfill and used the filter(...)[0] pattern instead. Feel free to revert that commit if you feel this is undesirable/unclear.

[smirzaei]

Everything looks good, thank you so much for this :)

Regarding the list vs object discussion, just think outside of currency formatting, in my case I remember having a small project using the data on this package to show the currency information to the client, and it needed to show multiple currencies at once.

Anyways, if anyone reports a performance problem, we'll definitely start optimizing :)

I'll do a minor version bump very soon.

[edorivai]

Cool, I'll update the docs later!

just think outside of currency formatting

I think there are probably better data sources out there to list currency information. Personally I would use this library purely for formatting. But this is all very nit picky ;).

In the end I'm also happy we're able to provide a backwards compatible update!

[smirzaei]

Yeah I agree, maintaining backwards compatibility in this update was pretty cool, good job :)