lemunozm / message-io

Fast and easy-to-use event-driven network library.
Apache License 2.0
1.11k stars 74 forks source link

Add try_recv() for non-blocking event queue reads #48

Closed RobDavenport closed 3 years ago

RobDavenport commented 3 years ago

Discussed in https://github.com/lemunozm/message-io/issues/46.

I tried to run rustfmt but ran into some issues with the .rustfmt.toml file. Probably a better idea to wait until that is fixed before merging this.

I also wrote some unit tests to cover some of the common use cases. Would appreciate some extra 👀 on it to make sure I've done it correctly. I'm sure there's a more elegant way of doing this but seems to serve as a solid foundation. Either way, feedback/comments appreciated!

Discussion points:

lemunozm commented 3 years ago

Sorry, I forgot the "instant" problem, let me check...

lemunozm commented 3 years ago

The problem is that the remove function takes the borrow of the instant that wants to remove. I saw that there is a function that allows performing exactly this but is nightly :(

Anyway, copy the instant is more or less as copy a number, so I think this copy is depreciable or maybe even optimized by the compiler.

As a minor minor really minor fix, you could move the copy inside the if and only copy when the condition was true:

if *next_instant.0 <= Instant::now() {
    let next_instant = *next_instant.0;
    return self.timers.remove(&next_instant);
}

But I think that for things like this the compiler will move it by us.

lemunozm commented 3 years ago

Regarding .rustfmt.toml. It has features of fmt nightly. I usually program in nightly to get the advantage of things like this but using always stable features.

RobDavenport commented 3 years ago

Thanks for the feedback - except an update within a short while while I make the changes!

lemunozm commented 3 years ago

Thanks, @RobDavenport for the contribution 😃 !!