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

Add async BufReader #1573

Merged
merged 1 commit into from
May 6, 2019
Merged

Add async BufReader #1573

merged 1 commit into from
May 6, 2019

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Apr 30, 2019

cc #1519

@taiki-e
Copy link
Member Author

taiki-e commented Apr 30, 2019

Blocked on #1533.

#1533 was merged.

use std::{cmp, fmt};
use super::DEFAULT_BUF_SIZE;

/// The `BufReader` struct adds buffering to any reader.
Copy link
Member Author

Choose a reason for hiding this comment

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

BufReader vs AsyncBufReader

(I have no strong opinion.)

Copy link
Member

Choose a reason for hiding this comment

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

Given it's a concrete type, and not a trait, I think BufReader is fine. It's similar to the naming scheme in Romio, Tokio, and Runtime 👍

Copy link

@ghost ghost May 6, 2019

Choose a reason for hiding this comment

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

I wonder if AsyncRead and AsyncWrite should've been named PollRead and PollWrite (because they only have polling read/write methods).

Then we could rename AsyncReadExt and AsyncWriteExt to simply Read and Write.

I don't see much point in the Async prefix if literally no other trait or type has it. Also, the Async* traits with polling methods are not indented to be used by end-users so it feels wrong for them to have simpler names than the *Ext traits which are used more often.

Copy link
Member

Choose a reason for hiding this comment

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

@stjepang that's an interesting point; perhaps opening a separate issue might be good for a discussion?

Copy link
Member

Choose a reason for hiding this comment

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

@stjepang the Async* traits are what should be used in bounds so likely used multiple times per file, whereas the *Ext traits should be imported and then be used via method syntax so only mentioned once per file.

Copy link
Member

Choose a reason for hiding this comment

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

Most relevant issue: #1398

@taiki-e taiki-e marked this pull request as ready for review May 3, 2019 00:54
@taiki-e taiki-e force-pushed the io-buf_reader branch 2 times, most recently from b69b486 to 1278df1 Compare May 4, 2019 01:29
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.

4 participants