RubyMoney / monetize

A library for converting various objects into `Money` objects.
MIT License
430 stars 107 forks source link

Junk strings end up being zero money #122

Closed flvrone closed 3 years ago

flvrone commented 5 years ago

'not a number'.to_money - results in zero money object, I believe it's a bug. What do you think?

antstorm commented 5 years ago

@FunkyloverOne hard to say. It definitely looks like a bug, but it's the way it is implemented. There's even a spec for it, look at the it 'does not return a price for completely invalid input'.

However I don't think that the parser should be as relaxed and should error when given anything that's even slightly off. I'm working on a strict parser that will do just that and if you wanna help out — give me a shout

flvrone commented 5 years ago

@antstorm Sure, I guess I could help you on it. I'm also working on a bunch of Mongoid goodies for money-rails meanwhile, and will open a new PR there soon. :)

As for this parser, I think it would be cool if you open your [WIP] PR here and mention me there, so I will be aligned with your progress.

antstorm commented 5 years ago

@FunkyloverOne sounds great, looking forward to those PRs. If you break them down into smaller chunks it will be easier for me to review 👍

I'll also wrap up my PR on the strict parser shortly and will tag you on it

softwaregravy commented 4 years ago

Also looking at this. Was there every a PR?

random example (at first I thought nil? was not working as expected). Switched to trying the example from the docs, which is working.

2.6.5 :001 > Monetize.parse("gibberish")
 => #<Money fractional:0 currency:USD>
2.6.5 :002 > Monetize.parse("gibberish").nil?
 => false
2.6.5 :003 > Monetize.parse("gibberish  howdy").nil?
 => false
2.6.5 :004 > Monetize.parse("gibberish  howdy") == nil
 => false
2.6.5 :005 > Monetize.parse("OMG 100")
 => nil
2.6.5 :006 > Monetize.parse("OMG")
 => nil
softwaregravy commented 4 years ago

another

2.6.5 :007 > Monetize.parse("not a valid value")
 => #<Money fractional:0 currency:USD>
antstorm commented 4 years ago

@softwaregravy thanks for pointing out 👍

I'm working on a new rewritten parser that should not have these issues — https://github.com/antstorm/monetize/tree/new-parser (last 2 commits)