Shopify / measured

Encapsulate measurements and their units in Ruby and Ruby on Rails.
MIT License
337 stars 28 forks source link

Better serialization and deserialization #148

Closed dvisockas closed 1 year ago

dvisockas commented 1 year ago

👋 Hi!

If we initialize a weight value and serialize it:

weight = Measured::Weight.new(0, :g)
serialized_weight = { value: weight.value, unit: weight.unit.name }.to_json
#=> '{"value":"0\/1","unit":"g"}'

If we then deserialize it:

deserialized_weight = JSON.parse(serialized_weight)
weight = Measured::Weight.new(deserialized_weight['value'], deserialized_weight['unit'])
#=> ArgumentError: invalid value for BigDecimal(): "0/1"

This is the case because Measured assumes that if the first argument is a string - it is parsed as a BigDecimal. Maybe it could be a bit smarter and try to parse the value as Rational if some condition (maybe just a simple \d+\\\d+ regex matched)?

kmcphillips commented 1 year ago

Hello! Yes this all makes sense. Thank you for the code sample clearly explaining the issue.

I think you identified the correct fix, or at least the first fix that I would try. Would you be willing to submit a PR? I can help you on it if you're stuck and would be happy to review and merge it (though I am on leave though and may not respond super fast).

I would approach it something like...:

WDYT?

dvisockas commented 1 year ago

I'd be happy to submit a PR. Will ping you if I get stuck!