Merging the two implementations?

[j-maas]

I took over rnon's ordered-containers package some while ago and randomly stumbled upon your ordered dict implementation. I was curious whether you were interested in merging the two implementations into one? I think this could make it easier for people to choose, but I'd also happily share maintership (which I didn't do much of anyway for now).

There are a few apparent differences that I see between ordered-containers and elm-ordered-dict.

• Your API is more minimal, while ordered-containers mirrors the entire Dict API (except for merge which is non-obvious on ordered dicts).
• Your API includes insertAt which sounds like a good addition to the API.
• You expose a type alias, whereas ordered-containers uses an opaque type.
• Your implementation looks more efficient (ordered-containers runs the updater multiple times and checks for member on the list instead of the dict).
• Your implementation remembers the position of elements that are removed with update. (I will open a separate issue for this.)

I think it sounds pretty straightforward to merge the two implementations into one. There are only minor design and API decisions would need discussion. I am also open to who would own the repository (you, me, a GitHub organization).

What do you think of a merge?

[wittjosiah]

I'm open to this if you want to take lead of it. I haven't been doing much Elm development recently and won't have time to get back into it for the foreseeable future but I'm happy to support a merge of the two libraries as they seem to accomplish essentially the same thing.

[j-maas]

I'm actually not that active either, but I presume that such a merge should be doable. I can work out the details and present them to you.

What I would suggest:

• Providing the full Dict-like API plus a toDict that returns a normal Dict, but not adding insertAt [1] and removing all "Combine" methods.
• Exposing only an opaque type, not an alias.
• Using the implementation that runs the updater fewer times.
• Completely removing items when they are set to Nothing via update.

I will make a branch on my repo implementing these changes and then showing it to you. Then you are free to make any suggestions you like, before we discuss how we actually merge the two libraries. :)

[1] I am unsure whether to include insertAt. It feels like a very natural extension to the API, but at the same time it feels like there would be other functions like moveFromTo missing. So I'd suggest being conservative and keeping it out of the API until a motivating use case emerges; unless you already have one?

[j-maas]

Ok, so I reworked the library in this PR. Feel free to comment on it!

Since I used your code as a guide, I'd like to add your copyright notice to my LICENSE file. Would you like to have Copyright (c) 2017, Josiah Witt included there? (Or something different?)

For the actual "merging", I'm considering moving the library to elm-community. This would solve multiple things: I've introduced breaking changes in the way the order is affected i. e. by inserting an existing key, and our two repos cannot be removed from the Elm package repository to prevent breaking the people who downloaded them. When the repository is moved to elm-community, I will add a notice to y0hy0h/ordered-containers stating that people should use the new library, link to it, and mention why it is better (more clearly defined handling of insertion order). Then, I'd publish a new breaking version to the Elm package repository to update the README there. I'd encourage you to do the same and can tell you when everything is ready for that.

What do you think?

[wittjosiah]

Seems reasonable to me! And the PR looks good 👍 No need to include the copyright notice verbatim; if you want to include attribution somewhere I'll leave that to your discretion.

[j-maas]

I've added a "History and contributors" section to the README. You can check it out if you want. ;)

I will proceed to clean up the documentation some more and then ask for inclusion in elm-community.

[j-maas]

It appears that the package does not fit nicely with the goals of elm-community, as can be seen the inclusion PR. So I'd like to publish a new major version of y0hy0h/ordered-containers. I think that is the easiest solution for the time being.

Please tell me if you have any reservations with this move forward.

[wittjosiah]

@y0hy0h sounds good to me 👍

Once that is published I will update the readme of this repo with a link to the order-containers package.

[j-maas]

Ok, I just published it: https://package.elm-lang.org/packages/y0hy0h/ordered-containers/latest/

Thank you so much for your kind cooperation! You made this really nice. :)

If there is anything new from elm-community that affects the link in the README, I'll make sure to contact you here again!