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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Classification of units #17

Open
emersonthis opened this issue Jul 18, 2019 · 3 comments
Open

Classification of units #17

emersonthis opened this issue Jul 18, 2019 · 3 comments

Comments

@emersonthis
Copy link

I have logic in place to prevent my users from trying to convert between incompatible units. For example: pounds --> gallons. I currently throw an error message, which explains that incompatible unit conversion is probably the issue. But I can't be certain, because the way I'm validating is by simply attempting the conversion and rescueing if it fails:

begin
    Measurement.parse("#{self.quantity} #{self.unit}").convert_to(self.ingredient.unit)
rescue
    errors.add(:unit, "is incompatible with associated ingredient unit (#{ingredient.unit}) in: #{ingredient.recipe.name}.  You are probably trying to mix weight and volume units.")
end

So this same validation error could also occur if a user attempted a nonsense unit. Ex: 37 blrblbs

I'm trying to figure out a way to be even more clear, and say something like:

"Impossible conversion! Pounds are a unit of weight but gallons are a unit of volume..."

I suspect that the library is already tracking this classification of units somewhere, but it does not appear to be exposed anywhere in the API? Would this be hard to do?

@mhuggins
Copy link
Owner

There is no context of "unit classification" built into the library, there is only a context of conversion calculation mappings between units. So if a mapping exists between two units, then it will convert using the calculation method provided.

Currently, if a calculation method does not exist, then an ArgumentError is raised. Similarly, if a unit does not exist, then an ArgumentError is raised. The error type is the same, but the message for the error is different: https://github.com/mhuggins/ruby-measurement/blob/master/lib/ruby-measurement/measurement.rb#L84-L94

One possibility would be to add explicit error classes such as InvalidUnitError and InvalidConversionError to cover these scenarios, which might help. The error class could specify the unit name or whatever else is needed to handle in a custom manner.

@emersonthis
Copy link
Author

One possibility would be to add explicit error classes such as InvalidUnitError and InvalidConversionError to cover these scenarios, which might help.

That sounds really helpful! And it looks like a pretty straightforward change to just add those more specific error classes. Would you like a PR for that?

@mhuggins
Copy link
Owner

That'd be great if you'd like to! One suggestion would be for them to extend ArgumentError so that it doesn't break existing implementations.

Thanks!

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

No branches or pull requests

2 participants