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

proposal: net: net.Listener.Iter #65520

Closed
leaxoy opened this issue Feb 5, 2024 · 8 comments
Closed

proposal: net: net.Listener.Iter #65520

leaxoy opened this issue Feb 5, 2024 · 8 comments
Labels
Milestone

Comments

@leaxoy
Copy link

leaxoy commented Feb 5, 2024

Proposal Details

Finally, we had Iterator api in std.

So I propose add net.Litener.Iter that returns an iter.Seq to produce net.Conn or error sequential.

The api may looks like:

type Result[T any] struct {
    data T
    err  error
}

func (r Result[T]) IsOk() bool { return r.err == nil }

func (r Result[T]) IsErr() bool { return r.err!= nil}

func (r Result[T]) Value() T {
    if r.err == nil {
        return r.data
    }
    panic("call value on error")
}

func (r Result[T]) Error() error {
    if r.err != nil {
        return r.err
    }
    panic("call value on data")
}

func Iter(l net.Listener) iter.Seq[Result[net.Conn]] {
	return func(yield func(result.Result[net.Conn]) bool) {
		for yield(result.FromPair(l.Accept())) {
		}
	}
}

While I personally don't like iter.Seq2 to express Either semantics,
because we don't succeed and fail at the same time, or neither succeed nor fail at the same time.

Strictly speaking, this situation should be expressed using the sum type, so the additional Result monad is introduced.

@leaxoy leaxoy added the Proposal label Feb 5, 2024
@gopherbot gopherbot added this to the Proposal milestone Feb 5, 2024
@panjf2000
Copy link
Member

Could you show us some use cases for your proposal? What drove you to make this proposal?

@panjf2000
Copy link
Member

cc @neild

@Jorropo
Copy link
Member

Jorropo commented Feb 5, 2024

IMO Something like Result is way too generic and would need it's own proposal.
You could also use 2 value iteration.

@leaxoy
Copy link
Author

leaxoy commented Feb 5, 2024

IMO Something like Result is way too generic and would need it's own proposal.
You could also use 2 value iteration.

Using product types to express sum types is an unreasonable thing in itself. I think we can't make any mistakes anymore. It's time to make some changes. Maybe this can be the first place to change.

I know there are currently some proposals related to the sum type, but nothing has changed for a long time and no one seems willing to move forward with them. The use of struct design here was forced to make some choices.

@earthboundkid
Copy link
Contributor

The convention in slices and maps is to call this “All” not “Iter”.

@neild
Copy link
Contributor

neild commented Feb 5, 2024

The proposed new iteration APIs in #61897 provide alternatives to existing functions which return a slice of results, permitting the caller to iterate over results without allocating the entire result slice.

net.Listener.Accept does not have have this flaw: It returns a single result at a time. It is not inconvenient to use. The proposed API here doesn't seem any simpler to use:

for {
  conn, err := listener.Accept()
  if err != nil {
    // handle error
  }
  // ...
}

vs.

for result := listener.Iter() {
  if result.IsErr() {
    // handle error
  }
  conn := result.Value()
  // ...
}

This is at most one line less code, or the same number of lines if we assign conn := result.Value().

Leaving aside the fact that Result really would need its own proposal, I don't think the added complexity of a second API for accepting connections can be justified here.

@adonovan
Copy link
Member

adonovan commented Feb 5, 2024

I wonder how many applications have more than a single call to Listener.Accept; extra syntactic convenience wouldn't help very often. And Accept is such a significant enough operation that perhaps it warrants being explicit within the loop, not hidden in an iterator.

@leaxoy
Copy link
Author

leaxoy commented Feb 6, 2024

Closing it, for no compelling reason other than to copy the same capabilities from rust.

And due to the lack of type system support for sum type, there has not been much change in usage.

@leaxoy leaxoy closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

7 participants