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

Add Constants module #19

Closed
ianmackenzie opened this issue Nov 1, 2018 · 5 comments · Fixed by #23
Closed

Add Constants module #19

ianmackenzie opened this issue Nov 1, 2018 · 5 comments · Fixed by #23

Comments

@ianmackenzie
Copy link
Owner

This would be a central place to store conversion factors, useful for a couple reasons:

@katjam
Copy link
Contributor

katjam commented Dec 21, 2018

@ianmackenzie Hoping to have a few hours to spare in the next week or so. I'll have a go at this one.

@ianmackenzie
Copy link
Owner Author

Excellent, sounds great @katjam! I'm currently working on #22, but I don't think that should conflict with this. Adding better product support will really only touch Quantity.elm (with the exception of changing the definition of Newtons in Force.elm from Rate Joules Meters to Product Kilograms MetersPerSecondSquared), whereas adding a constants module will touch basically everything except Quantity.elm.

I may end up pushing out a new major release soon with the expanded product support - I don't think it really makes much of a difference whether adding a constants module gets merged in before or after that happens. (Although perhaps not right before...)

@katjam
Copy link
Contributor

katjam commented Dec 21, 2018

Agree. Sounds unlikely that we'll tread on each other's toes. Toying with name stuff. I guess we've agreed Constants.elm as the module - but would SharedConstants.elm (if we aren't pulling all of them out) or ConversionFactors.elm be any more explanatory?

Unless you say otherwise, I'll assume we:

@ianmackenzie
Copy link
Owner Author

The module name might depend on what pattern we use for the actual constant names - for example I could see either Constants.foot (a constant representing the length of one foot in SI units) or ConversionFactors.metersPerFoot/ConversionFactors.feetPerMeter. The second seems more explicit although the first is a bit more concise/composable, e.g.

cubicFoot =
    foot * foot * foot

instead of

cubicMetersPerCubicFoot =
    metersPerFoot * metersPerFoot * metersPerFoot

Probably worth playing around with a few different styles and see what seems best (both at call sites and within the module itself), but I think I'm leaning slightly towards the second ("there are worse things than being explicit"!). I think you'd end up with both 'direct' and 'inverse' conversion factors in ConversionFactors.elm, like

module ConversionFactors

metersPerFoot : Float
metersPerFoot =
    0.3048

feetPerMeter : Float
feetPerMeter =
    1 / metersPerFoot

(or perhaps feetToMeters and metersToFeet?) which might be used like

module Length

import ConversionFactors

feet : Float -> Length
feet numFeet =
    meters (numFeet * ConversionFactors.metersPerFoot)

inFeet : Length -> Float
inFeet length =
    inMeters length * ConversionFactors.feetPerMeter

Lastly, while I think it makes sense to start with the shared constants, I think my preference would be to eventually move all constants/conversion factors into the single module (for consistency, and so it's easier to check all of them against a reference).

@katjam
Copy link
Contributor

katjam commented Dec 21, 2018

Sounds like we are thinking along the same lines. Maybe both natural over-thinkers. I'll see how I get on one step at a time, leaving the PR open for comment.

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 a pull request may close this issue.

2 participants