Shopify / measured

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

Make Conversion immutable #47

Closed thegedge closed 7 years ago

thegedge commented 7 years ago

I see little to no value in having the ability to add units to Measured::Conversion after it's been created. Immutability again will give us some nice benefits here, in particular with how I'd approach https://github.com/Shopify/measured/issues/32 (increased performance).

I think a DSL or configuration step would be really nice here, which is basically how we use it today. I'll throw out a couple of options:

Close to what we have today

class Measured::Length < Measured::Measurable
  conversion.configure do
    case_sensitive true

    base :m, aliases: [:meter, :metre, :meters, :metres]

    unit :cm, value: "0.01 m", aliases: [:centimeter, :centimetre, :centimeters, :centimetres],
  end
end

DSL

Measured::Length = Measured::Measurable.build("Length") do
  case_sensitive false

  base :m, aliases: [:meter, :metre, :meters, :metres]

  unit :cm, value: "0.01   m", aliases: [:centimeter, :centimetre, :centimeters, :centimetres]
  unit :mm, value: "0.001  m", aliases: [:millimeter, :millimetre, :millimeters, :millimetres]
  unit :in, value: "0.0254 m", aliases: [:inch, :inches]
  unit :ft, value: "0.3048 m", aliases: [:foot, :feet]
  unit :yd, value: "0.9144 m", aliases: [:yard, :yards]
end
kmcphillips commented 7 years ago

I'm pretty sure the ("Length") is unnecessary.

Also, the build shouldn't be on the Measured::Measurable object because then it's pulled around through every base class instance (Weight, Length, etc).

Maybe more:

Measured::Length = Measured.build do
  base :m, aliases: [:meter, :metre, :meters, :metres]

  unit :cm, value: "0.01   m", aliases: [:centimeter, :centimetre, :centimeters, :centimetres]
  unit :mm, value: "0.001  m", aliases: [:millimeter, :millimetre, :millimeters, :millimetres]
  unit :in, value: "0.0254 m", aliases: [:inch, :inches]
kmcphillips commented 7 years ago

Also need to make sure that the call to build creates a subclass of Measurable, and that trying to subclass it without use of the DSL will fail too.

kmcphillips commented 7 years ago

Either that or remove the superclass entirely and make it a mixin.