-
Notifications
You must be signed in to change notification settings - Fork 761
Common polymorphic base class #168
Comments
You raise an interesting question, but the main problem is that the socket type (HTTP or HTTPS) has to be decided at compile time. This means that for instance One could create a Server interface but this interface could not contain for instance the resource map. This interface would in other words not be very useful other than setting config values and running |
I don't have the HTTPS code incorporated yet, but to avoid this issue when I get to it, I've abstracted the web server completely to the point where there is essentially just a This message can be passed around my application. The
The worker pool is also in these files. I do my resource mapping a little further up in my workers (that function call is here: https://github.com/opset/openset/blob/master/src/http_serve.cpp#L90). The maps are defined here: https://github.com/opset/openset/blob/master/src/rpc.h#L122-L154 and the function that dispatches to functions that do stuff in my app are here: https://github.com/opset/openset/blob/master/src/rpc.cpp#L2352-L2375 |
Hi, Sorry for long reply time. The goal was to make a REST/JSON server which hides HttpServer implementation but allows anyway to give a custom instance (this is justconcept code) :
Here is the pseudo-code of my generic server (sorry it is from at least 1 year old version of your project). The required generic features are :
Since I manage to do it, do you think you could also do it ? Thank you, best regards ! |
I just precise that my ServerSOA system already works but I broke compatibility so I can't use your updates for now unfortunately |
I have definitely needed to add common functionality and use pointers and refs to clients and servers where a non-template base class would help. I vote Yes if it makes sense for the project at some point. Thanks eidheim! |
Thank you all for great feedback, and sorry for being absent in this thread. I have however given this some thought from time to time. The problem as I see it is that If someone has a solution for this, a PR is most welcome, or at least enlighten me on how this could be solved:) |
Thinking intial thought out loud... maybe use stream and socket pointers in
the base, init to 0, and only created by derived class as needed? Or
accessors, eg virtual asio::ssl::stream* get_stream() { return 0; } in
base. Very rough thought.
…On Thu, Jan 25, 2018 at 11:53 AM, Ole Christian Eidheim < ***@***.***> wrote:
Thank you all for great feedback, and sorry for being absent in this
thread. I have however given this some thought from time to time.
The problem as I see it is that ServerBase::Response::send is dependent
on ServerBase::Connection that again has socket_type socket. Thus the
Response class would need to be built at compile time depending on if you
use HTTP or HTTPS and could not be part of a non-template base class. This
again makes it impossible to put the resource handlers in a non-template
base class as well. Now if asio::ssl::stream
<http://www.boost.org/doc/libs/1_66_0/doc/html/boost_asio/reference/ssl__stream.html>
and asio::ip::tcp::socket
<http://www.boost.org/doc/libs/1_66_0/doc/html/boost_asio/reference/ip__tcp/socket.html>
shared a base class this issue could be solved, but they do not as far as I
can see.
If someone has a solution for this, a PR is most welcome, or at least
enlighten me on how this could be solved:)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABt1YK7WZnaa8O5VaHRx9cWy33VUa1sdks5tOLGcgaJpZM4QeqOv>
.
--
news <https://bitpost.com/news> | trade <https://abettertrader.com/> | blog
<https://bitpost.com/blog>
|
I ended up tackling this over the last couple of days. What I did was to eliminate the templating on ServerBase and its related inner-classes. I derived an Http/HttpsConnection class and Http/HttpsResponse class to store the specifics. I then pulled out the uses of sockets (asio:ssl::stream and asio:ip:tcp::socket) and put them in virtual methods, which are called by ServerBase. The extensive use of lambdas caused a number of headaches, but I think I got them worked out. I really didn't change any of the executable code, just where it lives. I've tested using my app (currently only HTTP) and using the http_examples.cpp and https_examples.cpp examples. You can see my changes here. If they look OK, I'll submit a pull-request. Hope this helps |
@ttislerdg Thank you! I'll have to study your changes in more detail, but it seems that you have found a solution that might very well have solved this issue. A pull request is most welcome, and we'll take it from there. |
I also made an attempt in https://github.com/eidheim/Simple-Web-Server/tree/base_class. Feedback is welcome. |
After some thought, I think I have come to the conclusion that making a non-templated base class complicates that source code too much with respect to what is achieved. Also keep in mind that for instance std::vector does not have a base class, so in a way the design patterns from the standard library is in a way currently followed instead. That said, one can still define the resource-handlers at one place and reuse them in both a HTTP and HTTPS server. For instance, this works using c++14: #include "server_https.hpp"
using namespace std;
using HttpServer = SimpleWeb::Server<SimpleWeb::HTTP>;
using HttpsServer = SimpleWeb::Server<SimpleWeb::HTTPS>;
int main() {
auto io_service = std::make_shared<boost::asio::io_service>();
HttpServer http_server;
http_server.config.port = 8080;
http_server.io_service = io_service;
HttpsServer https_server("server.crt", "server.key");
https_server.config.port = 8081;
https_server.io_service = io_service;
auto default_resource = [](auto response, auto /*request*/) {
response->write("Hello World");
};
http_server.default_resource["GET"] = default_resource;
https_server.default_resource["GET"] = default_resource;
http_server.start();
https_server.start();
io_service->run();
} This issue is still however open for discussion:) |
Hello |
Or maybe using a factory, like :
|
I found some interesting discussion around OOP/Templates here: https://stackoverflow.com/questions/1039853/why-is-the-c-stl-is-so-heavily-based-on-templates-and-not-on-interfaces. I'm not agreeing with everything that is said in the replies for this stackoverflow-question, but I think a healthy balance between OOP, template and functional programming can be achieved. Regarding the original issue post, here is one solution using templates: #include "server_https.hpp"
using namespace std;
using HttpServer = SimpleWeb::Server<SimpleWeb::HTTP>;
using HttpsServer = SimpleWeb::Server<SimpleWeb::HTTPS>;
template <class T>
void setup_and_start(T &server) {
server.config.port = 8080;
server.default_resource["GET"] = [](auto response, auto /*request*/) {
response->write("Hello World");
};
server.start();
}
int main() {
bool use_https = false;
if(use_https) {
HttpsServer server("server.crt", "server.key");
setup_and_start(server);
}
else {
HttpServer server;
setup_and_start(server);
}
} On the plus side, using templates simplifies the library code. This might seem like a small argument, but in an open source project like this it is important that potential contributors can understand the code. On the other hand, template programming makes it harder for static analysing tools to identify problems, and IDE tooling is lacking when it comes to making sense of code in template functions and classes. |
I think most users of the code, that is, developers who need to use an
http/https library, would prefer the interface be as simple as possible.
For me, that would mean encapsulating everything in one easy interface, and
making as invisible as possible any difference between http and https. A
few parameters and away you go. Having to actually have a different
template parameter (as opposed to driven entirely by variables) makes use
of the library less convenient in some cases. That seems to be a hard
metric to override.
On the other side of the coin, I think your IMPLEMENTATION with templates
is extremely elegant - probably the most elegant solution possible. It
would be a major change to have to give up that elegant implementation. So
there's the rub. Not sure of the conclusion! Just mostly confirming the
conflict. :-)
…On Fri, Mar 23, 2018 at 9:55 AM, Ole Christian Eidheim < ***@***.***> wrote:
I found some interesting discussion around OOP/Templates here:
https://stackoverflow.com/questions/1039853/why-is-the-
c-stl-is-so-heavily-based-on-templates-and-not-on-interfaces. I'm not
agreeing with everything that is said in the replies for this
stackoverflow-question, but I think a healthy balance between OOP, template
and functional programming can be achieved.
Regarding the original issue post, here is one solution using templates:
#include "server_https.hpp"
using namespace std;
using HttpServer = SimpleWeb::Server<SimpleWeb::HTTP>;using HttpsServer = SimpleWeb::Server<SimpleWeb::HTTPS>;
template <class T>void setup_and_start(T &server) {
server.config.port = 8080;
server.default_resource["GET"] = [](auto response, auto /*request*/) {
response->write("Hello World");
};
server.start();
}
int main() {
bool use_https = false;
if(use_https) {
HttpsServer server("server.crt", "server.key");
setup_and_start(server);
}
else {
HttpServer server;
setup_and_start(server);
}
}
On the plus side, using templates simplifies the library code. This might
seem like a small argument, but in an open source project like this it is
important that potential contributors can understand the code. On the other
hand, template programming makes it harder for static analysing tools to
identify problems, and IDE tooling is lacking when it comes to making sense
of code in template functions and classes.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABt1YGFFJZ3m6rYq_ne9y8mZ5IAFZxp4ks5thP7tgaJpZM4QeqOv>
.
--
news <https://bitpost.com/news> | trade <https://abettertrader.com/> | blog
<https://bitpost.com/blog>
|
Hi
Will it be possible to have a base class without template for being able to create a polymorphic instance ?
For now it is impossible to do something like this :
For now I extracted a part of the code in a new class called
HttpServerGeneric
but so I broke compatibility with your code...Thanks
Gabriel
The text was updated successfully, but these errors were encountered: