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

Removing the RingBufReader and RingBufWriter types #4

Closed
fhartwig opened this issue Feb 22, 2015 · 2 comments
Closed

Removing the RingBufReader and RingBufWriter types #4

fhartwig opened this issue Feb 22, 2015 · 2 comments

Comments

@fhartwig
Copy link
Contributor

This is really a question more than an issue:
Could we remove the RingBuf Reader and Writer newtypes (and implement Buf and MutBuf directly on RingBuf)? I don't actually understand what purpose they have, apart from disambiguating method names from Buf and MutBuf (but that could be done in other ways).
If we could get rid of them, that would massively improve the ergonomics of RingBuf, at least for my use case. There are two problems that I have with them:

  • There are about a dozen instances in my code where I have to write something along the lines of let r = buf.reader(); let s = r.bytes(); where I would like to write let s = buf.bytes() instead. (Writing let s = buf.reader().bytes() doesn't work because the returned reader would not live long enough). This is admittedly only a minor annoyance.
  • More importantly, I have a struct containing a RingBuf and I would like to implement a method returning a slice into that RingBuf. I can't do that, though, since the slice I want to return cannot outlive the RingBufReader that I have to create in that method. This is a bigger problem than the first one, since I don't think I can work around it.

I'd be interested in an explanation of the necessity of the newtypes or any thoughts you have on the subject. If you think we could get rid of them, I'd be happy to do the implementation work.

@carllerche
Copy link
Member

The answer is yes. Having separate types is a relic of an old limitation that has since been fixed. I would accept a PR that does the work 😄

@carllerche
Copy link
Member

Fixed by #5

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

No branches or pull requests

2 participants