Split into multiple headers for development and merge for distribution

[moonshadow565]

It would be really helpfull to split the library into multiple .hpp files for development and then merge them into single .hpp / .h for distribution :)
Examples:
https://github.com/ThePhD/sol2/blob/develop/single.py
https://github.com/oftc/jsoncpp/blob/master/amalgamate.py
https://github.com/connormanning/arbiter/blob/master/amalgamate.py

This would not only allow for easier development but also for possible PRs/contributions or simple modifications/extensions that others might need to make.
It would also improve code reuse and abstraction mechanisms.
For example sprintf_socket could be made as a method writefmt of Stream and not implementation detail.

[yhirose]

@moonshadow565, thanks for the suggestion. Yes, I understand it has a number of benefits. But I still prefer keeping it in one file which doesn't depend on any other process such as merging or building process. My other 'cpp-???' libraries (cpp-peglib, cpp-linenoise, cpp-mmaplib...) also follow the same principle.

For example sprintf_socket could be made as a method writefmt of Stream and not implementation detail.

This is actually a nice code improvement. I'll implement it when I have time. Thanks!

[moonshadow565]

Tbh the only upside i can see with single file vs single folder is that its slightly easier to download.
Its a simple python script which works for both python 2 & 7.
And it wont depend on anything because you can simply put single header version in dist folder in the repo.
You could even use appveyor to automate the process or as only mainteiner invest a second of your time for merging.
So it would still follow the same principle :)
That being said, what i wanted to is add simple support for websockets.
It would have save me much scrolling if it were bit split in multiple headers.
Not asking you to add it for me (even tho you can if you want lol).
Most of the job is already done (cross platform sockets and parseing http response).
But it would save me some trouble if implementation details were moved to their respective classes.
Short list of what i would like:
-Headers as class that inherits from map with case insesitive hash
-write/ operations not as detail but either a member function or free functions with common name write/read using stream as first argument or << operator
-urlencode/decode as utility classes
I think this covers 90% of your detail namespace. xD
I hope you don't find me anoying :)

EDIT:
You should also probably rename socket_reader to stream_reader since it uses sockets and not streams.
Noticed that you convert in multiple places between const char* and std::string multiple times (potenionaly makeing uncessary copies).
You would save yourself some time by using only std::string there.

[yhirose]

It would have save me much scrolling if it were bit split in multiple headers.

I understand it's painful. 😢 (I guess I like the @antirez's style which contains everything in a single .c file like linenoise.c and kiko.c. Of course he doesn't use the style for large scale projects such as redis.)

That being said, what i wanted to is add simple support for websockets.

Glad to hear that! @underscorediscovery also mentioned he could implement it at this comment. Since all my projects using cpp-httlib don't actually need websocket right now, it would be nice if we could enable the websocket feature with the define symbol CPPHTTPLIB_WEBSOCKET_SUPPORT .

But it would save me some trouble if implementation details were moved to their respective classes.

A number of libraries in C++ Boost library uses detail namespace style to hide implementation details from users. I followed the same style. (I admit that the current detail namespace in httplib.h becomes like a kitchen sink...) But I want to expose only necessary classes such as Server, Client, Request and Response. (I am even thinking to move Stream under detail namespace.) Of course there could be better ways to improve the structure of the library. But I'll start thinking about it when the library gets really big and getting out of my control and comprehension.

Headers as class that inherits from map with case insesitive hash

I totally agree it should be done. I have just made the issue #32.

You should also probably rename socket_reader to stream_reader since it uses sockets and not streams.

True. I renamed it to be stream_line_reader that represents what it does.

Noticed that you convert in multiple places between const char* and std::string multiple times (potenionaly makeing uncessary copies).

Great find! Could you make a new issue specifying the places that cause the unnecessary copies?

For example sprintf_socket could be made as a method writefmt of Stream and not implementation detail.

Thanks! I renamed it to stream_write_format.

I hope you don't find me anoying :)

No problem at all. 😄 It's actually good for me to hear from others, so that I could look into the library from different perspective. Even though I may not take all suggestions from people, I always appreciate such thoughts. Of course since this is my personal project, I can spend time only when I am available. Thanks for your patience!

[moonshadow565]

Ty for your time.
I understand now clearly why you keep it single file and wont press into that anymore.
Would be nice to make file server optional as well since that might not be needed allways as well.

I understand purpose of details namespace :)
Was just questioning the decision if you want to hide some of realy usefull functions there.
Url encode/decode functions are one such functions for example.
As well as that it looks messy as you said already.
If you move stream functions there then you could move some of stream/socket helpers to member functions or give them common name and use ADL to chose which one to use.

I will be trying to add websockets there when i get time.
Don't have any specifics on string issue but i think its sometime thats worth checking out.
c_str() will check for missing NULL terminator on the end the string() so thats potentional allocation and resize.
I'm sure that headers use string, so maybe start from them?
@yhirose should close the issue