Improve parsing delimiters

[RubenIgnacio]

I refactor the delimiter detection logic to improve overall readability and structure. I moved the enforce_currency_delimiters check to the beginning of the process to ensure that when the option is enabled, the currency's delimiters are respected immediately before any automatic detection logic is attempted.

[yukideluxe]

Thanks for trying to make this complicate code better! I have some thoughts 🙆🏻‍♀️

[yukideluxe]

There's a behavior change here! Before, the Monetize.enforce_currency_delimiters was only checked when the used delimiters were just 1.

In main, if we do Monetize.parse('4,567.89', 'EUR') (parsing euros with usd delimiters) was returning <Money fractional:456789 currency:EUR> which is OK but it technically doesn't "enforce" the currency delimiters like the setting implies to do 😬

In your changes, if we do Monetize.parse('4,567.89', 'EUR'), it returns <Money fractional:457 currency:EUR>. It removes the currency thousands separator (which is . in this case) which converts the number to 4.56789 or, for Spaniards like me, "4,56789".

@RubenIgnacio I am a bit torn about this change. In one hand it does make sense because the setting does say "ENFORCE" and, in the other hand, the parsing before seemed to be "better"? Imagine an American trying to input an euro amount. There doesn't seem to be any explanation about why they added it only for one delimiter apart from fixing a "bug" #57. We don't even know how many people use this 😬

I can be convinced if you have more insight than I do but, if we do this, we should definitely add something in the CHANGELOG about this behavior change. Something like:

**Behavior change**: Changed `Monetize.enforce_currency_delimiters` behavior. Parsing numbers with two delimiters was not enforcing the currency delimiters. Inputs using delimiters that don't match the currency's configuration may be parsed differently than before when `enforce_currency_delimiters` is set to `true`. For example, parsing `Monetize.parse('4,567.89', 'EUR')` will return `<Money fractional:457 currency:EUR>

An alternative is renaming the variable to something more specific so it's clear the heuristic changes for just one delimiter 🙏🏻

Ping @sunny for his thoughts!

PS: note to self, or whoever wants to take this, improve the test suite when enforce_currency_delimiters is true/false 😳

[yukideluxe]

There doesn't seem to be any explanation about why they added it only for one delimiter

Maybe it's just obvious than when there are two different delimiters, the last one is going to be a "decimal" mark no matter what? It does make sense to me 😇

[RubenIgnacio]

I agree, and renaming the variable might be the best solution. I found this configuration during the refactor, but I couldn't find any reference to it in the documentation for monetize or money, only the comments in the monetize.rb file and a mention in the changelog for version 1.4.0.

I also found it curious that the original code only applied to cases with a single delimiter. However, I can restore it to its original location if it causes any issues.

[sunny]

Good catch @yukideluxe! Although not being really enforced, I’d rather we keep the previous behavior as it is more flexible. 🙏🏻

[yukideluxe]

@sunny agreed! @RubenIgnacio let's keep it the way it was! it would be nice to add a test for the example I've given above 😳 We can circle back to this at another time! Your refactor is nice no matter what! 🙏🏻

[RubenIgnacio]

Hi, I've restored the previous behavior and added some additional tests for the enforce_currency_delimiters case.

@yukideluxe @sunny, do you think there are any other edge cases that should be covered?