Shopify / measured

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

Add support for extending Numerics with #unit methods #58

Closed thegedge closed 6 years ago

thegedge commented 7 years ago

Requiring this new file will result in adding extensions to Numeric that allows one to more easily create Measurable instances. For example:

irb(main):001:0> require 'measured/util'
=> true
irb(main):002:0> Magic = Measured.build do
irb(main):003:1*       base :magic_missile, aliases: [:magic_missiles]
irb(main):004:1>
irb(main):005:1*       unit :fireball, value: "2/3 magic_missile", aliases: [:fire, :fireballs]
irb(main):006:1>     unit :ice, value: "2 magic_missile"
irb(main):007:1>     unit :arcane, value: "10 magic_missile"
irb(main):008:1>     unit :ultima, value: "10 arcane"
irb(main):009:1>   end
=> Magic
irb(main):010:0> 1.kg
=> #<Measured::Weight: 1.0 kg>
irb(main):011:0> 5.5.metres
=> #<Measured::Length: 5.5 m>
irb(main):012:0> BigDecimal("1.2345678").arcane
=> #<Magic: 1.2345678 arcane>

Closes https://github.com/Shopify/measured/issues/39

thegedge commented 7 years ago

Not sure how to test this without polluting Numeric in our test environment, which I'd be okay with, but wanted some input first.

kmcphillips commented 7 years ago

Not sure how to test...

Look into ActiveSupport::Testing::Isolation with both forking and subprocess: http://api.rubyonrails.org/classes/ActiveSupport/Testing/Isolation.html

kmcphillips commented 7 years ago

Also would need to make sure we're only adding lower case methods, without duplicating or overriding any existing methods, and that we may not want all aliases or pluralizations.

It's a big complex change.

These core extensions are polarizing and hard. It's one of the main reasons we didn't use some other libs.

thegedge commented 7 years ago

Much of this doesn't let you opt out of one or the other. Having the requires for length/weight at the bottom of the util file means you get all or none. Would be nice to be able to say "I want weight extensions, but I rarely use distance" or whatever.

That's why in #39 I was hinting towards having this as part of the DSL, since it gives you a little more control over what to opt in. I'm not a big fan of a configure block just for this, unless there was something else we can imagine configuring. Maybe specifying a default case sensitivity value too?

thegedge commented 7 years ago

Oh yeah, another thing that's annoying is that you get Measured::Length and Measured::Weight without asking. It makes it hard to do anything like this, because you have to account for whether or not they've already been constructed when doing any configuration like this. If we included a "default case sensitive" config value, it wouldn't apply to those two, unless we rebuilt them (but then we'd have to reassign constants).

I'd like to propose making them not included by default.

kmcphillips commented 7 years ago

Definitely 100% not included by default.

thegedge commented 6 years ago

Closing out this one, since it's been awhile. There's an issue opened if someone wants to tackle adding something opt-in :)