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

Jump to bottom

proposal: io: add ReaderFunc and WriterFunc #47678

Open
dsnet opened this issue Aug 12, 2021 · 10 comments
Open

proposal: io: add ReaderFunc and WriterFunc #47678

dsnet opened this issue Aug 12, 2021 · 10 comments
Labels
Proposal Proposal-Hold
Projects
Proposals (old)
Milestone
Proposal

Comments

@dsnet
Copy link
Member

dsnet commented Aug 12, 2021

I propose adding the following:

type ReaderFunc func([]byte) (int, error)
func (f ReaderFunc) Read(b []byte) (int, error) {
    return f(b)
}

type WriterFunc func([]byte) (int, error)
func (f WriterFunc ) Write(b []byte) (int, error) {
    return f(b)
}

In tests, I find it common that I need an io.Reader or io.Writer with specialized behavior. Usually, this is only necessary for a single test case within a large table-driven test. In order to have a customized io.Reader, I need to declare a concrete type to implement the Read method. Since you can only declare a type with methods at the top-level, this results in the declaration potentially being located far away from the code actually using it.

The proposed ReaderFunc (and WriterFunc) allows me to create an io.Reader that 1) requires no named declarations, and 2) have the specialized Read behavior inlined with the place that actually plans on using it.

Example usage:

// Cancel the request after the first read to abruptly end the stream.
var calls int
ctx, cancel := context.WithCancel(context.Background())
req, err := http.NewRequest("POST", url, io.ReaderFunc(func(b []byte) (int, error) {
	if calls > 0 {
		cancel()
	}
	calls++
	return copy(b, []byte("whatever")), nil
}))
req = req.WithContext(ctx)

This a library-specific workaround to the several proposed language changes (see #21670, #25860, #47487, and many others). Being a non-language change, it is presumably a more palatable change than a full language change. I personally can only recall ever needing something like #21670 for io.Reader and io.Writer.

This is very similar to the existing http.HandlerFunc type.

\cc @josharian
@gopherbot gopherbot added this to the Proposal milestone Aug 12, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 12, 2021
@ianlancetaylor
Copy link
Contributor

See also #47487 which tries to address this issue more generally.

@peterbourgon
Copy link

io.Reader/Writer both require something from/into which the reads/writes will occur. This is a natural semantic fit to an interface method, which must be defined on a type that would presumably serve that need. But functions are semantically stateless, so for this to make sense the function would necessarily have to close over some state, at which point you're acting as an ersatz method, anyway. (This is structurally different than e.g. http.HandlerFunc, which can fulfill its requirements statelessly.)

Minor value delivered, substantial confusion generated. -1.

@josharian
Copy link
Contributor

io.Reader/Writer both require something from/into which the reads/writes will occur.

This is not true. I’ve written Readers/Writers that block forever, generate data forever, always return errors, etc.

the function would necessarily have to close over some state

Closures can be entirely local; types with methods cannot. This can help readability.

@dsnet
Copy link
Member Author

dsnet commented Aug 13, 2021 •
edited

Closures can be entirely local; types with methods cannot. This can help readability.

The example in the original post shows a function that closes over two variables, both on the lines immediately above it. Had that been declared as a new type, it would have been located very far from where it was actually used. That's a pretty big hit to readability.

@peterbourgon
Copy link

This is not true. I’ve written Readers/Writers that block forever, generate data forever, always return errors, etc.

That's true. io.Discard is a stateless io.Writer; rand.Rand a stateless (ish) io.Reader. My language was too severe. Still, they're exceptional cases.

Closures can be entirely local; types with methods cannot. This can help readability.

Yep, it can! I acknowledge the value this would deliver. I'm saying that value is outweighed by the costs to coherence.

@mvdan
Copy link
Member

mvdan commented Aug 13, 2021

I'd prefer if we found a general solution like #47487 - but if that proposal is rejected, I think we should accept this one.

@renthraysk
Copy link

Would also be useful for interface hiding, for example when using io.Copy family of functions.

io.Copy(dst, ReaderFunc(f.Read))

Would be safe from an io.WriterTo implementation. And here's os having to do the same thing.

https://github.com/golang/go/blob/master/src/os/file.go#L161-L167

@jfesler
Copy link

jfesler commented Aug 16, 2021

I too prefer #47487 - but I don't think I want to for an unknown number of years for it.

In the meantime, the code outlined by @dsnet can be used in projects today, just in a different name space from "io", so this particular proposal isn't a significant blocker.

@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

I will put #47487 into the regular proposal minutes and move this one to hold.

@rsc rsc moved this from Incoming to Hold in Proposals (old) Aug 18, 2021
@rsc
Copy link
Contributor

rsc commented Aug 18, 2021

Placed on hold.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Hold
Projects
Proposals
Status: Hold
Proposals (old)
Hold
Milestone
Proposal
Development

No branches or pull requests
9 participants
@josharian @rsc @peterbourgon @jfesler @ianlancetaylor @mvdan @dsnet @gopherbot @renthraysk