elm-community / dict-extra Goto Github PK

According to https://github.com/elm-community/Manifesto/blob/master/maintainers.md, no maintainer is assigned for this package.

@Skinney, are you willing to take that role?

The role of a maintainer is briefly explained here: https://github.com/elm-community/Manifesto/blob/master/README.md#the-role-of-a-maintainer

When the function given to mapKeys returns the same value multiple times, the last returned values overwrites all values that came before. I think it would be good to add a sentence to the docs of mapKeys so users keep this in mind.

As per this: elm-community/guidelines#27.

This was useful to me more than once. Would it warrant inclusion in this library?

frequencies ["a","b","c","b","c","b"]
--> {"a": 1, "b": 3, "c": 2}

frequencies : List comparable -> Dict comparable Int
frequencies list =
    list
        |> List.foldl
            (\el counter ->
                Dict.get el counter
                    |> Maybe.withDefault 0
                    |> (\count -> count + 1)
                    |> (\count -> Dict.insert el count counter)
            )
            Dict.empty

In my opinion, it's more intuitive to group elements in the order they appear in the original list. Here are some examples of groupBy implemented in other languages. Notice that the sort order is preserved within each group:

Ruby (standard library)

[2,3,4,5,6,7].group_by { |x| x % 2 }
# => {0=>[2, 4, 6], 1=>[3, 5, 7]}

Javascript (lodash)

_.groupBy([2,3,4,5,6,7], x => x % 2)
// {"0":[2,4,6],"1":[3,5,7]}

This issue can be fixed by simply using a foldr instead of foldl:

groupBy : (a -> comparable) -> List a -> Dict comparable (List a)
groupBy keyfn list =
-    List.foldl
+    List.foldr
        (\x acc ->
            Dict.update (keyfn x) (Maybe.map ((::) x) >> Maybe.withDefault [ x ] >> Just) acc
        )
        Dict.empty
        list

In a call groupBy keyfn list, with the current implementation, the function keyfn will be applied potentially many times to each element of list, though one time should be enough. Depending on how expensive keyfn is to compute, this can be very bad.

The situation is like https://github.com/elm-lang/core/issues/485, and the solution would also be similar to there, i.e., first making a pass through the input list and pairing each element with its keyfn-image.

As a separate note: There was some discussion elsewhere, elm-community/elm-list-extra#38 (comment), whether this function should actually live in what is now https://github.com/elm-community/list-extra.

It would be nice to have a version of groupBy that also maps the values. This seems like a common scenario when taking apart and reassembling dictionaries, as below.

invert : Dict comparable (List comparable1) -> Dict comparable1 (List comparable)
invert dict =
    let
        f ( x, ys ) =
            List.map (\y -> ( x, y )) ys

        g _ xys =
            List.map Tuple.first xys
    in
        dict
            |> Dict.toList
            |> List.concatMap f
            |> Dict.Extra.groupBy Tuple.second
            |> Dict.map g

Come to think of it, inverting dictionaries is also pretty common.

Per discussion at https://elmlang.slack.com/archives/elm-community/p1460684154000129 by @eeue56 it turns out that trying to install the package Skinney/elm-dict-extra fails with a message saying "Error: Could not download source code successfully.". Although github redirects https://github.com/Skinney/elm-dict-extra to this repo it seems that elm-package does not follow that redirect.

We have reached out to @Skinney via email to ask that he fork this repo back again to re-establish the Skinney/elm-dict-extra repo and thereby satisfy elm-package.

We need to update the "repository" field value to be "https://github.com/elm-community/elm-dict-extra.git".