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

NewFromFloat panics with invalid/unrecognized currency input, unlike New #144

Closed
hubarthurcoelho opened this issue Apr 5, 2024 · 1 comment

Comments

@hubarthurcoelho
Copy link
Contributor

Description

The NewFromFloat function panics when provided with an invalid currency code, due to an attempt to access .Fraction on a nil pointer. This behavior is inconsistent with the New function, which handles invalid currency inputs by falling back to a default currency. This may lead to unexpected crashes in applications relying on the NewFromFloat function for currency conversion when invalid currency codes are inadvertently passed, or not yet registered in the library.

Steps to Reproduce

  1. Use NewFromFloat with an invalid/unrecognized currency code.
  2. Observe the panic due to a nil pointer dereference when attempting to access .Fraction.

A minimal code example demonstrating the issue is available here: Go Playground

Expected Behavior

Ideally, NewFromFloat should have a consistent invalid/unrecognized currency handling strategy with New, possibly by defaulting to a "safe" currency configuration when the currency code is not recognized, instead of panicking.

Possible Solution

A straightforward fix could be aligning the way currencies are set up in currencyDecimals during instantiation with how it's done in New. Essentially, we'd be replicating the currency creation logic.

I'm not sure if this is a known issue, or even if this is actually expected behavior, but if it's not, it should be an easy fix... I would gladly open a PR for this one!

@Rhymond
Copy link
Owner

Rhymond commented Apr 8, 2024

👋 @hubarthurcoelho,
Lovely find!

A straightforward fix could be aligning the way currencies are set up in currencyDecimals during instantiation with how it's done in New. Essentially, we'd be replicating the currency creation logic.

Yes, that would be the fix, I would really appreciate if you could open the PR, looking at the code it doesn't look like it should cause any backwards compatibility issues

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

No branches or pull requests

2 participants