Skip to content

Conversation

seanmonstar
Copy link
Member

@reem here's what I'm thinking to support arbitrary underlying Streams. Being generic removes the hit of a dynamic lookup, but can also be less ergonomic. Though, with the default type parameters, a Server, or Request, etc can still be used, with HttpStreams.

fixes #5

@reem
Copy link
Contributor

reem commented Sep 8, 2014

I quite like this as a solution, but I'm also wary about how this makes impls for Handler more complex. I think it's a worthy tradeoff, but it's worth consideration. The second problem is that helpers which receive Request and Response will likely just use Request and Response, inadvertently making them unusable with alternative streams.

@seanmonstar
Copy link
Member Author

Yea, both of those problems crop up. Is there some other way to do this?

@reem
Copy link
Contributor

reem commented Sep 8, 2014

I think not. That dynamic dispatch is so much more ergonomic is, in my opinion, a slight problem in rust today. The only way to make this really work would be to remove the default type parameter in everything except Server.

@seanmonstar
Copy link
Member Author

So, make everyone deal with the generics?

It's probably worth comparing benches... the only other way would be to accept a Box<Reader> and Box<Writer> in the constructors, right? I have not looked into DST at all, but that recently landed, and allows having Trait objects without indirection...

@reem
Copy link
Contributor

reem commented Sep 8, 2014

Benchmarking dynamic vs. static dispatch here would probably be wise as it would let us know if this is even worth worrying about. If we could get static dispatch with dynamic-dispatch-like ergonomics, that would be even better.

@reem
Copy link
Contributor

reem commented Sep 10, 2014

Merged as part of #29

@seanmonstar seanmonstar deleted the network-stream branch September 10, 2014 16:18
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.

Mocking Request and Response
2 participants