janet-lang / spork

Various Janet utility modules - the official "Contrib" library.
MIT License
117 stars 35 forks source link

Added abstract type set #144

Closed tionis closed 1 year ago

tionis commented 1 year ago

Based on work done by @ianthehenry in janet.guide (which he said he considers public domain). This implements a simple mutable set based on the JanetTable data structure. This is mainly a draft to ask for opinions on differentiating between mutable/immutable sets (like done for other data structures in janet) and also a few of the pain points listed below:

ianthehenry commented 1 year ago

I would expect for compare to only compare the pointers of the data types, since it's a mutable structure -- that would be consistent with the behavior for tables and buffers and arrays.

We could simplify this a little bit by adding some private wrap and unwrap functions on the abstract type that return the underlying table, and then implement functions like symmetric-difference and friends in Janet code instead of C. (I have nothing against C).

Also worth noting that this does rely on sort of a private implementation detail of the Janet GC. We could fix that, at the cost of an extra pointer indirection. I don't have a good sense for how fragile it is.

tionis commented 1 year ago

Just a small note: As soon as I find the time I will split up the implementation into an immutable and mutable version mirroring structs and tables regarding the hashing behavior. (I will also add a function to convert between the two) I'm unsure on how to label the namespace, probably something like set/mutable/new and set/immutable/new etc., but I'm not really sure yet. This naming scheme seems too verbose and inelegant.

bakpakin commented 1 year ago

Not sure this is really something that useful as it is no more efficient than a Table, a good example for how to make abstract types but not something I would actually use.

Also some questionable behavior with set_get - it looks as though you can't put numbers in the set?

tionis commented 1 year ago

Valid points. Also important is that the additions of helper functions like map-keys did reduce the benefit of this quite a bit. I also don't have time at the moment to complete this, so I'll close this.