Skip to content

Conversation

@schurhammer
Copy link
Contributor

Issue gleam-lang/gleam#1262

Still WIP but creating PR for feedback.

Reference Implementation
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/PersistentHashMap.java

@schurhammer
Copy link
Contributor Author

One question is where is isEqual and inspect implemented? I couldn't find the source files.
I think there might need to be some changes to these since the same set of entries could potentially have a different internal node structure, especially if/after items had been deleted from the map.

@schurhammer
Copy link
Contributor Author

Thanks inoas :)

We somehow need to get the new Map class into prelude or find a workaround where we don't need to do that. I'm not sure which path to take here. Some options:

  1. implement the new Map in prelude instead of stdlib
  2. make ther new Map a subclass of the native JS Map, then it should be handled by those code paths in theory
  3. support a magic "equals" method, similar to the "inspect" method (prelude.js:283)

@lpil
Copy link
Member

lpil commented Jul 24, 2022

3 sounds like it would be useful for other future user implemented data structures as well as this one. Let's go for that one.

I could imagine us in future promoting the JavaScript map implementation into core, but let's be conservative and keep it in the standard library for now.

@schurhammer schurhammer marked this pull request as ready for review August 13, 2022 12:28
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful work! I've left some questions

@lpil
Copy link
Member

lpil commented Dec 22, 2022

Hey @schurhammer ! I finally got to the bottom of my github inbox now that I'm full time on Gleam 😁

Is this something you would like to continue with?

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incredible work, thank you so much! I am really super impressed and grateful 💜

@lpil
Copy link
Member

lpil commented Mar 13, 2023

I've rebase this into main

@lpil lpil closed this Mar 13, 2023
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 this pull request may close these issues.

3 participants