Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom units via Unitwise#register #37

Merged
merged 11 commits into from
Apr 28, 2017
Merged

Conversation

joshwlewis
Copy link
Owner

@joshwlewis joshwlewis commented Apr 22, 2017

There have been numerous requests for adding one-off, custom, or unsual units to Unitwise. I've not supported these requests in the past because:

  1. This gem is an implementation of UCUM, which curates a canonical source of units, and any units added to this gem are likely to conflict (or may in the future).
  2. I don't want to maintain a list of units and their conversion values. This is exactly what UCUM exists to do.

However, there is value in supporting user-defined units. Some domains have legitimate units that don't make sense in UCUM.

This PR supports user-defined units via Unitwise.register(). It takes a hash of atom properties. For example, if your app had to measure bourbon in fingers ("three fingers of whiskey, please"):

Unitwise.register(
  names: ["finger", "fingers"],
  symbol: "馃",
  primary_code: "fng",
  secondary_code: "fng",
  scale: {
    value: 1.0,
    unit_code: '[foz_us]'
  }
  property: 'fluid volume'
)

And now you can convert to and from it:

Unitwise(1, "gallon").to_fingers

@dpritchett
Copy link

dpritchett commented Apr 23, 2017

The code reads smoothly and the docs are even better. My only question is how user-friendly the failure cases are if say someone tries to specify subtly incorrect params to :register. Will they get a nice hand-holding failure response that makes it obvious where to go next? I can't tell just by reading this PR.

 Unitwise.register(
   names: ["finger", "fingers"],
   symbol: "馃",
   scale: {
     value: 1.0,
     unit_code: '[foz_us]'
   },
   property: 'fluid volume'
 )


Unitwise::AddRegisterError: "Lol u forgot poland 馃嚨馃嚤.  And :primary_code. And :secondary_code."
... stacktrace goes here ...

>

Maybe something like that?

@dpritchett
Copy link

I'm also wondering if you might want to (not now, YAGNI) add a set of self.list_custom_units introspective functionality one day if people start adding in custom packs of made up stuff for video game minmaxing or some such wildness.

@joshwlewis
Copy link
Owner Author

Ok, thanks for the feedback. I went ahead and added Unitwise::Atom#validate! which Unitwise.register uses. Any errant input should now be caught quickly.

As for listing custom units, we have Unitwise.search, so that's probably good enough for now.

@joshwlewis joshwlewis merged commit 07e61df into master Apr 28, 2017
@joshwlewis joshwlewis deleted the atom-registration branch April 28, 2017 03:10
@pinkynrg
Copy link

pinkynrg commented Aug 3, 2017

I was wondering if it might be worth implementing some check if unit already exists in the atom list. Maybe by searching for primary/secondary code? In case it exists we should either throw an exception "atom already registered" or just a plain message "atom updated!" in case we want it overridable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants