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

Add batch processing functions for fiat and satoshi conversions #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d4rp4t
Copy link

@d4rp4t d4rp4t commented Dec 5, 2024

This PR introduces batch processing utilities to enhance the efficiency of fiat and satoshi conversions. By minimizing the number of API calls, these changes aim to optimize performance for scenarios involving multiple simultaneous conversions.

Changes in this PR:
Added batch processing functions:

  • getBatchFiatValue: Calculates fiat values for an array of satoshis.
  • getBatchSatoshiValue: Calculates satoshi values for an array of fiat amounts.
  • getBatchFormattedFiatValue: Formats multiple fiat values as localized strings.

@rolznz
Copy link
Contributor

rolznz commented Dec 6, 2024

@d4rp4t I am not sure this is really needed in this library as this is more custom and not necessarily required. The new methods you added can be done by the application, right?

@d4rp4t
Copy link
Author

d4rp4t commented Dec 6, 2024

@d4rp4t I am not sure this is really needed in this library as this is more custom and not necessarily required. The new methods you added can be done by the application, right?

You're absolutely right, this can be implemented at the application level.
My main goal with this PR was to reduce unnecessary API calls, especially in scenarios like rendering lists or handling large datasets. By incorporating this functionality into the library, we can encourage a more efficient approach that helps prevent overloading the API.

@rolznz
Copy link
Contributor

rolznz commented Dec 10, 2024

@d4rp4t on the other hand, by adding these methods we also complicate and increase the API surface and does not necessarily mean a user of the API would use them. I think this is a very specific use-case, so I still think this should be handled at the application level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants