Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Currency list we are passing in config as string can be atoms #55

Closed
pkrawat1 opened this issue Dec 27, 2017 · 3 comments
Closed

Currency list we are passing in config as string can be atoms #55

pkrawat1 opened this issue Dec 27, 2017 · 3 comments

Comments

@pkrawat1
Copy link
Member

pkrawat1 commented Dec 27, 2017

Currently, we are passing currency as a string "USD" in the application config :global_config.

This can be improved to use atoms like :usd or something which can be driven and controlled by the library instead of a plane string.

@pkrawat1 pkrawat1 added this to the Release 1.6.0 milestone Dec 27, 2017
@oyeb
Copy link
Contributor

oyeb commented Dec 28, 2017

Atoms are best suited for keys (in a map or keyword), as far as I've read.
We'll have to convert these atoms to upcase strings when making requests to the gateway anyways. There's a thread on elixirforum discussing serialisation and atoms.

@SagarKarwande
Copy link
Member

As there is limit on atoms count in erlang. link

  • Though using atom for currency/country code will not increase atom count at great extent. But huge project may consider huge atom count for using library.
  • Also some gateway are using atoms for keys in the post body creation. So should we use string or atom there.

@pkrawat1 pkrawat1 removed this from the Release 1.6.0 milestone Jan 25, 2018
@oyeb
Copy link
Contributor

oyeb commented Jan 25, 2018

This is now obsolete because of #62 and #71.
It's the Money lib's responsibility to provide a sane/nice interface to create Money objects.

@oyeb oyeb closed this as completed Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants