search

hyperium / http

Rust HTTP types
Apache License 2.0
1.16k stars 291 forks source link

add StatusCodeClass #508

Closed ede1998 closed 3 years ago

ede1998 commented 3 years ago

StatusCode has a method is_code_class for each status code class but there is no method to determine the status code class itself. This PR adds an enum StatusCodeClass as well as a method StatusCode::class(&self) -> StatusCodeClass.

I wrote without investing much time. If you are open to accept my change, I can go over it again to make it prettier.

seanmonstar commented 3 years ago

Thanks for the PR! The original code did have a concept of a StatusClass (well, before it was taken from hyper), but we opted not to include since the extra type doesn't seem to be that useful. Are the workflows where such a type would have been significantly better than the StatusCode::is_blah() methods that already exist?

ede1998 commented 3 years ago

Thanks for the answer! I dug around a bit in the history. I think you're referring to #1.

Are the workflows where such a type would have been significantly better than the StatusCode::is_blah() methods that already exist?

I wanted to match on the StatusCodeClass and was just surprised that it didn't exist. That's about it. In the end I also just needed if code.is_success() { ... } else { ... }. However, if you actually need to handle the different classes differently, you can only match on the code.as_u16() or build an if-chain, which is a bit annoying.

If there was only StatusCode::class(), I think both use cases (match on many and just check for a single class) would be met.

Having said that I don't think it's worth the effort (esp. removing the existing methods). If you really want to match, it's not hard to do it yourself (maybe also with an extension trait).

seanmonstar commented 3 years ago

Yea, you found it! OK, cool, sounds about what I thought. It could be useful, but adding to the API space may not carry it's weight. In that case, I'm going to close :)