Greetings to all.

I have spent the last couple of evenings learning about Rust and trying it out. Wrote a simple cli calculator as a first thing and thought I would improve it by making it available over http.

I was actually a bit surprised to find that there was no http tooling in the standard library, and searching online gave me an overload of information on different libraries and frameworks.

I ended up implementing my own simple HTTP server, might as well as this is a learning project.

Now I have it working, and while it isn’t perfect or done, I thought that this would be a good time to check what things I am doing wrong/badly.

Which is why I am here, would love to get some pointers on it all to make sure I am going in the right direction in the future.

The project is hosted here: https://github.com/Tebro/rsimple_http

  • jameseb@lemm.ee
    link
    fedilink
    English
    arrow-up
    6
    ·
    1 year ago

    A few things I noticed:

    • In http::request::parse(), do you actually need a BufReader? It would be better to make it generic over something implementing BufRead, 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.
    • In 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.
    • Consider whether you need a special to_string() method at all, or whether you can just implement Display (which gives you to_string() for free via the ToString trait).
    • You are using 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 implement Error and Display and it will fit fine into the rest of the error handling infrastructure. There are crates like thiserror that can reduce the boilerplate around this.
    • You have an io.rs that doesn’t appear to be connected to anything.
    • You have a 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

    • tebro@lemmy.tebro.fiOP
      link
      fedilink
      arrow-up
      3
      ·
      1 year ago

      Thanks for the great points.

      • Using the BufRead trait sounds like a good improvement!
      • Yes, this is a stupid temp thing that I have to fix once I get better errors in place. Which you also had some good ideas on :)
      • Good idea, should be helpful
      • As mentioned above, this sounds great!
      • Yup, left over from the initial CLI application
      • Yeah it is there as an example, need to look into how examples are better set up
      • jameseb@lemm.ee
        link
        fedilink
        English
        arrow-up
        2
        ·
        edit-2
        1 year ago

        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 the src directory (and in its own subfolder if it has multiple files), and you can add an entry for it in the Cargo.toml (although it should automatically detect it if you put it in the examples directory, so that is only strictly necessary if you want to change the default settings).

        • tebro@lemmy.tebro.fiOP
          link
          fedilink
          arrow-up
          1
          ·
          1 year ago

          Yeah I moved it over and it got a lot nicer, nice to have this type of thing built in to cargo.