search

eidheim / Simple-Web-Server

A very simple, fast, multithreaded, platform independent HTTP and HTTPS server and client library implemented using C++11 and Boost.Asio. Created to be an easy way to make REST resources available from C++ applications.
MIT License
2.61k stars 751 forks source link

Status code refactoring #157

Closed knowledge4igor closed 6 years ago

knowledge4igor commented 7 years ago

I have improved (from my point of view) code in status_code.hpp. Please consider this pull request.

eidheim commented 7 years ago

I kept the library header only for simplicity and backward compatibility, but the status_codes() function is a good reason to add source files for utility, crypto and status_code header files. Then the status_codes vector will only be initialised once across translation units (which is a better solution than both mine and yours currently).

Also, I do not think map is needed in this case, since the number of status codes is low. Adding a two way map is also slightly more resource demanding.

eidheim commented 7 years ago

But thank you for looking into these header files, I am glad we have more eyes on them. And I am open for discussion about keeping this library header only, or if we should split the above mentioned header files into header/source pairs (keep in mind that boost.asio/asio is header only, and the compilation time increase of keeping this library header only as well is barely noticeable) . Hopefully, we can decide on this before the upcoming major v3 release.

knowledge4igor commented 7 years ago

Yes, the definition of the status_codes can be added to cpp file. As for vector vs. map, I don't think that using loop over the vector is an optimal solution for the lookup, even keeping in mind that the amount of items is low. You will use a lot repeated iterations of loop on each status_code call. When you use map you have to fill this map but after that you have O(log(n)) complexity lookup. Ok, it up to you to choose the best solution for this part of project. What about adding tests for status_codes() (#156) ?

eidheim commented 7 years ago

I'll think some more on this, but either way I'll merge the tests soon. Thank you for these as well.

eidheim commented 6 years ago

I optimised the status code functions in https://github.com/eidheim/Simple-Web-Server/commit/647a733251535ef936fdb49430ef33d79110d0e6 much like you suggested. One difference in my commit is that the various containers are initialised once across translation units meaning you only have one initialisation. This is achieved through inline functions and static declarations.

I'll add that I did some tests before making these changes, and std::unordered_map and map was faster than using vector. Thank you again.