Skip to content

Conversation

@Izzette
Copy link

@Izzette Izzette commented May 8, 2025

@Izzette Izzette marked this pull request as draft May 8, 2025 14:10
@Izzette
Copy link
Author

Izzette commented May 8, 2025

I'd like to make add an All method to the structures themselves instead. I'm not sure if the iterator interfaces and sequence wrappers should be exported. I'll mark this as ready for review when I decide.

I also need to add a test case for the yield function returning false.

Copy link

@markuspeloquin markuspeloquin left a comment

Choose a reason for hiding this comment

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

I have no association with this project. I was just thinking about using it until I saw it doesn't have proper iterators.

High level:

  • Don't provide iterators for builders
  • Rename AllWithIndex to All2, matching Seq2
  • Remove the asserts (which I'm guessing you planned on doing anyway)
  • Maybe also upgrade the version in go.mod (instead of coping the Seq/Seq2/Ordered interfaces), it's not like Go 1.22 is supported anymore

I also do wonder if a more direct version would be better, rather than an adapter over Iterator. Like copy it, take out the parts you don't need, and simplify it until it looks like Seq.

// This can be used to perform range-like operations on the [iter.Seq] and inter-operate with other libraries using the Go 1.23 iterable function API.
//
// TODO(Izzette): could this use a better name?
func IndexedSeq[T any](it IndexedIterator[T]) Seq[T] {

Choose a reason for hiding this comment

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

It'd be nice if you didn't rely on interfaces for this for performance critical code. You can accomplish this with generics.

func IndexedSeq[T any, Iter indexedIterator[T]])(it Iter) Seq[T] {

In this case, the indexedIterator interface is used a constraint.

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.

Consider supporting golang 1.23 iterators

2 participants