Fix readability warnings #1722
Fix readability warnings #1722
Conversation
httplib.h
Outdated
| // TODO: check if Content-Length is set | ||
| return false; | ||
| return (req.method == "POST" || req.method == "PUT" || |
There was a problem hiding this comment.
This is incorrect. I intentionally put the if statement before the TODO comment.
There was a problem hiding this comment.
Ah, I see. Something will come after the first check. I will revert this part.
httplib.h
Outdated
| @@ -868,15 +868,16 @@ class Server { | |||
| bool routing(Request &req, Response &res, Stream &strm); | |||
| bool handle_file_request(const Request &req, Response &res, | |||
| bool head = false); | |||
| bool dispatch_request(Request &req, Response &res, const Handlers &handlers); | |||
| bool | |||
| static bool dispatch_request(Request &req, Response &res, | |||
There was a problem hiding this comment.
I don't like to put 'static'. It's less readable.
There was a problem hiding this comment.
I understand your feeling, but having a static method means the this pointer is not passed as an automatic parameter. This will reduce the call overhead a little bit and ensure the calling instance won't affect the outcome or be affected by any side-effect.
httplib.h
Outdated
| @@ -8361,7 +8363,7 @@ inline SSLClient::SSLClient(const std::string &host, int port, | |||
| : ClientImpl(host, port, client_cert_path, client_key_path) { | |||
| ctx_ = SSL_CTX_new(TLS_client_method()); | |||
|
|
|||
| detail::split(&host_[0], &host_[host_.size()], '.', | |||
| detail::split(host_.data(), &host_[host_.size()], '.', | |||
There was a problem hiding this comment.
If we do host.data(), should we do the same to &host[host.size()] like host_data() + host.size()? This change actually creates inconsistency and made the code less readable...
There was a problem hiding this comment.
Good point, I overlooked this case.
There was a problem hiding this comment.
I am ok with the notation like &a[0]. IMHO, most of C/C++ programmers know what it means, and it's a common notation.
There was a problem hiding this comment.
I agree with you, the meaning is very well-known. In this particular case, it even makes more sense to use this notation because of the begin/end parameters of detail::split() for iteration. In the other cases, the meaning really is to put data into a vector's buffer.
|
@jfsimoneau I basically don't agree half of changes like adding |
Did not fix readbility-qualified-auto, will do a separate pull request
jfsimoneau
force-pushed
the
fix-readability
branch
from
7d5d4f6
to
5afcdb0
Compare
|
Could you leave only changes for adding missing |
|
Sure, on it! |
|
Replaced all I did notice most |
|
Thanks for all the improvements.
When I have time, I'll take a look at the code and thing about what I can do. Thanks again for your fine contribution! |
Did not fix readbility-qualified-auto, will do a separate pull request