Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iter: document general guidance for writing iterator APIs #71901

Open
mvdan opened this issue Feb 22, 2025 · 9 comments
Open

iter: document general guidance for writing iterator APIs #71901

mvdan opened this issue Feb 22, 2025 · 9 comments
Labels
Documentation Issues describing a change to documentation.

Comments

@mvdan
Copy link
Member

mvdan commented Feb 22, 2025

While https://pkg.go.dev/iter does a good job at explaining the basics of iterators, it leaves out a few important bits of information which may be really useful when writing APIs with iterators. The two most important of them being:

  1. Whether an exported func or method should itself be an iterator, or return an iterator.

The consensus seems to be to export funcs which return iterators, e.g. func (*Foo) Bars() iter.Seq[Bar] used like for bar := range foo.Bars(), rather than func (*Foo) Bars(yield func(Bar) bool) used like for bar := range foo.Bars. This is a bit more consistent with cases where one needs to supply parameters to obtain an iterator, as then the iterator must be a return parameter.

See #66626 (comment), for example, where @adonovan originally proposed adding methods to go/types which were directly iterators.

  1. How errors should be returned to the caller.

If an iteration can fail with an error, it's not obvious whether to return one top-level error, like func Foo() (iter.Seq[Bar], error), or to provide an error at each iteration step, like func Foo() iter.Seq2[Bar, error]. Arguments can be made either way, but I think fundamentally one can implement any reasonable semantics with either signature.

The original proposal at #61897 seemed to clearly favor iter.Seq2[Bar, error] via its func Lines(file string) iter.Seq2[string, error] example, yet none of the value-error examples or text have survived into the final godoc we have today.

As of today I think there is no clear consensus for errors; as recently as last October it was still being discussed as part of a new API proposal.

There may be other API conventions that the iter godoc should mention as well, but these two seem like the most important to me. I would suggest that we document these guidelines sooner than later, so that iterator APIs across the Go ecosystem can be reasonably consistent and predictable.

@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Feb 22, 2025
@thepudds
Copy link
Contributor

thepudds commented Feb 22, 2025

yet none of the value-error examples or text have survived into the final godoc we have today.

FWIW, that was a conscious decision. See for example the commit message here:
https://go-review.googlesource.com/c/go/+/591096

(There was a separate comment on the rationale that I couldn’t dig up immediately, but I think in short it was something like it wasn’t 100% obvious if it was the right idiom, and I think also an element of “let’s first see how people use it for real”).

@gabyhelp
Copy link

Related Discussions

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@dsnet
Copy link
Member

dsnet commented Feb 22, 2025

I understand the sentiment to deliberately not document until we first get experience, but I also agree with @mvdan that we should eventually (hopefully sooner rather than later) guidance on what to do. Doing something like iter.Seq2[Bar, error] is what I've been leaning towards lately.

@jub0bs
Copy link

jub0bs commented Feb 22, 2025

@mvdan

If an iteration can fail with an error, it's not obvious whether to return one top-level error, like func Foo() (iter.Seq[Bar], error) [...]

That error result can communicate that iterator instantiation failed, but it cannot communicate that iteration itself fails. You'd need something like func Foo() (iter.Seq[Bar], func() error) (which was proposed in some other issue), wouldn't you?

@jub0bs
Copy link

jub0bs commented Feb 22, 2025

Another point of clarification that would be welcome: categorisation of iterators. I feel that differentiating single-use iterators from all other iterators was a poor choice. "Stateless" iterators (i.e. iter.Seq[Foo] and iter.Seq2[Bar,Baz] that have no free variables) vs. all others would be more useful.

An analogy that comes to mind is linear systems vs nonlinear systems in control theory:

the theory of nonlinear systems is like a theory of non-elephants

@gazerro
Copy link
Contributor

gazerro commented Feb 22, 2025

@jub0bs

You'd need something like func Foo() (iter.Seq[Bar], func() error) (which was proposed in some other issue), wouldn't you?

func Foo() (iter.Seq[Bar], func() error) was proposed by @jba (70084 comment), and it is indeed a convincing alternative to func Foo() iter.Seq2[Bar, error]:

iter, errf := Foo()
for x := range iter {
    ...
}
if err := errf(); err != nil {
    return err
}

@jub0bs
Copy link

jub0bs commented Feb 22, 2025

@gazerro Yes, that's the issue I was thinking of. Thanks. I'm not sure I'd be in favour of promoting this approach, though.

@mvdan
Copy link
Member Author

mvdan commented Feb 22, 2025

Oops, of course, I meant to write something like that - so that an error (or a wrapped list of errors) can be returned once the iterator is done.

@ianlancetaylor
Copy link
Member

This discussion is why we don't have general guidance about how to return errors. People don't yet agree.

I think a commonly known method, such as All, should return an iterator rather than be an iterator. That ensure consistency of the common method across different types. This is mentioned (briefly) at https://go.dev/blog/range-functions#standard-push-iterators. But I don't know that we need a convention for methods that are specific to a given type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation.
Projects
None yet
Development

No branches or pull requests

8 participants