Support 1.23 iterator for set

[amikai]

Hi @deckarep, I want to contribute the PR for supporting 1.23 iterator. I don't know if you are interested.

Background

The Iterator implemented in this package returns a channel to support range syntax. Therefore, it requires calling Stop to close the underlying channel. With the 1.23 iterator, we can use the for-range syntax without needing to call Stop. See the prototype implementation in issue #140.

My original thought was to use for-range on the Each method as follows. However, the logic is reversed. In the Each(func(t) bool) method of the set interface, if the passed function returns true, the iteration stops. According to the 1.23 SPEC, if the loop body for the range function terminates, the yield function will return false.

// not work as expect
mySet := mapset.NewSet[int]()
for v := range mySet.Each {
    ...
}

Proposal

Let's allow users to use the for-range loop to iterate through the set without additional operation (such as Stop) after version 1.23.

mySet := mapset.NewSet[int]()
for v := range Values(mySet) {
  ...
}

#140 is one of implementation to achieve that.
The pros of this PR: use func(func(element T) bool as iterator instead of 1.23 iter.Seq. So the go.mod no need to upgrade to 1.23. The user use 1.23 compiler can user for-loop. The user before 1.23 can use old function way.
The cons of using func(func(element T) bool) instead of iter.Seq is reduced readability.

The naming of Values follows the conventions used in the slices package slices.Values and the maps package maps.Values.

[deckarep]

Hello @amikai - this looks great.

• Since Go did not have language supported, built-in iterators all of my solutions were less than ideal.
• I agree it would be nice to use the Each function already provided but I understand that the logic is reversed.
• The naming convention of Values looks good to be in line with the stdlib.

I see no reason to not implement this and it looks like great support. I'd like to leave this PR open a little longer in case others watching the project have input.

[adambaratz]

My only note would be that the method should be called All because sets only contain values: https://groups.google.com/g/golang-nuts/c/le8CBGTK9-E/m/ieAF0llbAAAJ

Otherwise, sounds great! I just came here to see if there was support yet, so glad to see others are thinking about this.

[adambaratz]

Actually, sorry, misread the original note here. I'm wondering whether it would be better to add an All method instead of having this as a package function. for v := range mySet.All() looks a little nicer, to my eyes anyway, than for v:= range mapset.All(mySet). I think the functions from the slices and maps packages make more sense because those types don't have methods.

[deckarep]

Even though Sets only contain Values I think it’s fine keeping the function name as Values vs All.

I like the idea of this falling in line with other collection iterators that may exist.

[adambaratz]

I was a little confused by that, too. Hence my starting that thread. But both slices and maps have keys and values, so they're different from sets (which only have values). Ian Lance Taylor is on the Go team at Google. If he says that a set should have an All method instead of a Values method, I'd take his word on it.

[deckarep]

Thanks I’m very familiar with Ian Lance Taylor, definitely a legend in the industry.

All still feels overly generic to me, I feel more inclined to use Values or even Elements. In the truest, mathematical sense a Set has elements.

This is one reason I like to leave PR’s open is getting feedback like this. Does anyone else have opinions on this? As they say naming things is hard but I prefer to be pragmatic and get it done.