this post was submitted on 17 Nov 2023
17 points (94.7% liked)
Rust
6009 readers
6 users here now
Welcome to the Rust community! This is a place to discuss about the Rust programming language.
Wormhole
Credits
- The icon is a modified version of the official rust logo (changing the colors to a gradient and black background)
founded 1 year ago
MODERATORS
you are viewing a single comment's thread
view the rest of the comments
view the rest of the comments
A few things I noticed:
http::request::parse()
, do you actually need aBufReader
? It would be better to make it generic over something implementingBufRead
, that allows what you have but also makes tests and examples easier since you wouldn't have to open a TCP connection just to do something that is essentially string parsing.http::response::Response::to_string()
, that match on lines 78-85 makes me uneasy, because you are silently changing the status code if it isn't one you recognise. It would be better to signal an error. It would be even better to just check when the status code is set (perhaps with a status code enum to list the ones you support, since what you have isn't all the defined codes) so that you can't fail when converting to a string.to_string()
method at all, or whether you can just implementDisplay
(which gives youto_string()
for free via theToString
trait).String
as an error type pretty much everywhere. The better approach is to create an enum representing all the possible errors, so that a user of your library can match against them. Make the enum implementError
andDisplay
and it will fit fine into the rest of the error handling infrastructure. There are crates likethiserror
that can reduce the boilerplate around this.io.rs
that doesn't appear to be connected to anything.main.rs
, which seems off in something that sounds like it should be purely a library crate. You probably want that to be an example or an integration test instead.That's all I could see with a quick look. In terms of general advice: remember to look at warnings, run
cargo clippy
, and look at the API guidelines.Thanks for the great points.
I'm glad my points we're helpful!
There is some documentation on examples in the Cargo book. The basic procedure is to put it in an
examples
directory alongside thesrc
directory (and in its own subfolder if it has multiple files), and you can add an entry for it in theCargo.toml
(although it should automatically detect it if you put it in theexamples
directory, so that is only strictly necessary if you want to change the default settings).Yeah I moved it over and it got a lot nicer, nice to have this type of thing built in to cargo.