Implement missing imperative Set functions
#168
Conversation
|
No description provided. |
Co-authored-by: Claudio Russo <[email protected]>
The idea is for these functions to have a consistent API if we eventually include them for other data structures. Perhaps we could have |
src/Set.mo
Outdated
| /// where `m` and `n` denote the number of values in `set` and `iter`, respectively, | ||
| /// and assuming that the `compare` function implements an `O(1)` comparison. | ||
| public func retainAll<T>(set : Set<T>, compare : (T, T) -> Order.Order, iter : Types.Iter<T>) { | ||
| // TODO: optimize? |
There was a problem hiding this comment.
Yeah, this looks quadratic to me.
a simple improvement might be to construct a local Set from iter so you can use contains not linear search for testing common membership.
Also setList could just be created as an ordinary array since we know the size of set up-front. Probably cheaper.
There was a problem hiding this comment.
I noticed that Rust, Kotlin, and a few other languages use a predicate instead of collection as an argument to this function, which seems both more flexible and better for performance. I'll update the PR to go with this, and we can revise from there.
There was a problem hiding this comment.
Updated, although it's a bit wonky having to pass both the compare and predicate functions. Perhaps this is another reason for prioritizing dataclasses or another language feature to remove those as soon as possible (before merging this into dfinity/motoko-base).
| /// Set.add(set, Nat.compare, 2); | ||
| /// Set.add(set, Nat.compare, 3); | ||
| /// let pureSet = Set.toPure(set); | ||
| /// assert(PureSet.contains(pureSet, 1)); |
There was a problem hiding this comment.
| /// assert(PureSet.contains(pureSet, 1)); | |
| /// assert(PureSet.contains(pureSet, Nat.compare, 1)); |
| /// pureSet := PureSet.add(pureSet, Nat.compare, 2); | ||
| /// pureSet := PureSet.add(pureSet, Nat.compare, 3); | ||
| /// let mutableSet = Set.fromPure(pureSet); | ||
| // assert(Set.contains(mutableSet, 1)); |
There was a problem hiding this comment.
| // assert(Set.contains(mutableSet, 1)); | |
| // assert(Set.contains(mutableSet, Nat.compare, 1)); |
| @@ -69,11 +69,11 @@ module { | |||
| todo() | |||
| }; | |||
|
|
|||
| public func intersect<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |||
| public func intersection<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |||
There was a problem hiding this comment.
I'll rename in my branch too.
| todo() | ||
| }; | ||
|
|
||
| public func diff<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | ||
| public func difference<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { |
| todo() | ||
| }; | ||
|
|
||
| public func diff<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | ||
| public func difference<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { |
There was a problem hiding this comment.
| public func difference<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |
| public func difference<T>(set1 : Set<T>, set2 : Set<T>, compare : (T, T) -> Types.Order) : Set<T> { |
| @@ -69,11 +69,11 @@ module { | |||
| todo() | |||
| }; | |||
|
|
|||
| public func intersect<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |||
| public func intersection<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |||
There was a problem hiding this comment.
| public func intersection<T>(set1 : Set<T>, set2 : Set<T>) : Set<T> { | |
| public func intersection<T>(set1 : Set<T>, set2 : Set<T>, compare: (T, T) -> Types.Order) : Set<T> { |
Adds the following functions to the
Setmodule:toPure()fromPure()addAll()deleteAll()(see Map.delete and pure/Map.delete have inconsistent semantics #160 for naming discussion)retainAll()