ianmackenzie / elm-units

Simple, safe and convenient unit types and conversions for Elm
https://package.elm-lang.org/packages/ianmackenzie/elm-units/latest/
BSD 3-Clause "New" or "Revised" License
85 stars 14 forks source link

Idea: general `in_` function #27

Closed harrysarson closed 4 years ago

harrysarson commented 5 years ago

elm-units has a lot of inMeters, inFeet, etc functions.

I was thinking of a way to generalise these functions and came up with this:

module Main exposing (main)

import Html
import Length
import Quantity exposing (Quantity)

in_ : (Float -> Quantity Float u) -> Quantity Float u -> Float
in_ func quantity =
    Quantity.ratio 
        quantity
        (func 1)

feet =
    Length.feet 7

inMeters =
    Length.inMeters feet

inMeters2 =
    in_ Length.meters feet
-- alternatively
-- feet |> in_ Length.meters

main =
    Html.text (Debug.toString inMeters ++ " " ++ Debug.toString inMeters2)

Just an idea I had :)

ianmackenzie commented 5 years ago

It's definitely a cool approach! I can see it being useful in some cases like dynamic conversions (e.g. a user selects the units to convert to from a combo box, and that maps to a particular function to pass to in_) but there are also some downsides:

As a result, my inclination is to leave this out until there are some real-world examples of code that would be simpler/more readable/more robust/more extensible using a generic in_ than the existing inFeet, inMeters etc. Thoughts?

harrysarson commented 5 years ago

Two ways to do one thing is definitely a bad thing, and you make a good point regarding performance.

The idea is nice conceptually - regarding performance it could reduce bundle size (unlikely)

ianmackenzie commented 5 years ago

I don't think it would have much of an impact on bundle size, since Elm's dead code elimination should remove any inFeet, inMeters etc. functions you don't actually use (and they're one-liners anyways). But it's a good point to bring up - I think in general I'm strongly biased towards execution speed instead of bundle size/load time (having spent most of my career working on C++ technical applications), so it's useful to be reminded that in a lot of cases it's load time that's more important.

Anyways, I think I'm inclined to close this for now - feel free to comment further if you think there's a reason to re-open (new use case, compelling new argument, etc.).

ianmackenzie commented 5 years ago

Reopening for discussion as there are a couple more advantages to this approach I don't think were considered. First, it could avoid having confusing function names like Pixels.inPixels, since instead you could write

screenLength |> Quantity.in_ Pixels.pixels

Also allows for converting to weird custom units pretty easily, like getting speed in millimeters per minute:

Speed.metersPerSecond 0.001
    |> Quantity.in_ (Length.millimeters >> Quantity.per (Duration.minutes 1))
--> 60
harrysarson commented 5 years ago

This approach would definitely be slower that the current one - the hope would be that the elm compiler of the future would be smart enough to optimise out the cost.

ianmackenzie commented 5 years ago

There are a couple of things that occurred to me recently which make me slightly less worried about performance here than I have been in the past: