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

Add Quantity.minimumBy and Quantity.maximumBy #45

Closed MartinSStewart closed 4 years ago

MartinSStewart commented 4 years ago

I've found a couple times that I'd like to do something such as

nearestPoint = endPoints |> List.Extra.minimumBy (Point2d.distanceFrom startPoint)

This isn't possible though since distanceFrom returns Quantity. Sometimes the points have a generic unit as well so I end up needing to subvert elm-units by writing

rawQuantity (Quantity.Quantity value) =
    value

nearestPoint = endPoints |> List.Extra.minimumBy (Point2d.distanceFrom startPoint >> rawQuantity)

This can be avoided if Quantity.minimumBy: (a -> Quantity number unit) -> List a -> Maybe a and Quantity.maximumBy: (a -> Quantity number unit) -> List a -> Maybe a are added.

ianmackenzie commented 4 years ago

Makes sense! When I created the original list manipulation functions in the Quantity module I really just copied the List API (which doesn't have minimumBy/maximumBy), but I agree that they would be very useful.

I note that elm-community/list-extra does have minimumBy/maximumBy, which made me wonder if there were any other functions there which also should be ported to Quantity. But the only other functions that require comparable entries are things like unique which I don't think really make sense when working with floating-point data.

ianmackenzie commented 4 years ago

Just took a stab at this but ran into a bit of ambiguity - what should minimumBy/maximumBy should return if multiple items in the list end up being tied (have the exact same derived Quantity)? It's not a problem for minimum and maximum since all the tied values will necessarily be the exact same themselves, but in your minimumBy example you could have multiple different points that have the exact same distance from the start point. I can see a few options:

Thoughts?

ianmackenzie commented 4 years ago

I personally like the "return a List" approach, but it would also be a bit more expensive - all the implementations I can think of either end up allocating some extra memory, or evaluating the given a -> Quantity number units function more than once per item.

MartinSStewart commented 4 years ago

Huh, I've never considered this edge case before. I've used List.Extra.maximumBy and List.Extra.maximumBy several times and never run into any issue with this potential ambiguity. For that reason I'm leaning towards "Return the first of the tied values". It just feels like this is minor enough of an issue that changing the type signature is a larger drawback. It might be a good idea to document this edge case in the function though.

List.Extra does have maximumWith : (a -> a -> Order) -> List a -> Maybe a though. Perhaps maximumBy : (a -> comparable) -> List a -> Maybe a was chosen because if a user is having trouble with ties, they can fall back on maximumWith.

ianmackenzie commented 4 years ago

OK, I'll go with "return the first of the tied values" then (with a note in the docs). I guess I could always add something like some allMinimumsBy and allMaximumsBy variants in the future that would return Lists instead of Maybes =)