Shopify / measured

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

Allow .parse to load from string #81

Closed kmcphillips closed 7 years ago

kmcphillips commented 7 years ago

I'm doing some work of loading weights from files, and I want to be able to have values in an input file:

weight: 10.5 kg

And then create a weight object out of it:

Measured::Weight.parse("10.5 kg")

It's way slower than taking in the number and the symbol, but we'd be parsing those strings from the file first anyway.

kmcphillips commented 7 years ago

Fantastic answers!

Splitting out the regex for readability is a wonderful idea. I'll do that, and deal with negatives and single period. I'll also test those cases.

I'll see about refactoring the Unit class to share this logic. I'm sure we can use a single parser, or at least a single approach. I really don't want to just do .strip.split(" ", 2) because I want to support 12cm as that's easy to parse but has no spaces.

kmcphillips commented 7 years ago

Ok, reping.

Since we parse in both the unit definition and now on instantiating a measurement, I extracted a Measured::Parser class. It's nice because it now encapsulates the regex. Split it up as suggested, with comments. Way built out what's in that regex. Added a ton of edge cases. Shared the parsing.

Some tests are duplicated, but I'm ok with that. Had to add a case for a unit with a space in it.

If it's too much I can split the PR up. But I think it's easy enough to follow.

kmcphillips commented 7 years ago

Oh and @jonathankwok I went back and forth on the , parsing and decided not to include it. I would be doing a value.gsub(",", ".") since the numeric parsing in ruby truncates after the comma. It doesn't align with the language, so we won't do it here.

kmcphillips commented 7 years ago

@jonathankwok much clearer regex comments now