Am 30.09.2013 14:39, schrieb Michael Gius:
On 09/29/2013 12:44 PM, Tommi Mäkitalo wrote:
Am 28.09.2013 17:21, schrieb Michael Gius:
...
Although the signature of the method changes, it will be API compatible.
So no change in user code is needed.
Easier than I thought. Speaks for efficient design and exceptions put to
good use.
It makes sense to do it that way and it's useful, but I think it goes
only half way to address the problem. Documentation is key here.
Please correct me if I am wrong, but the problem seems to be that the
function name, a verb, implies an action that the function does not
execute. On a higher level, for those that did some network programming,
connect implies the 3-way TCP handshake.
Ok - I pushed just the changes.
I think the best way to avoid the problem in the future, is to document
this function thoroughly, as it's workings are, to some degree, open to
interpretation.
In that regard this problem raises other questions for me, as I am just
about to start digging into cxxtools.
For instance a quick grep into the source shows me that there is support
for keepalive. But without digging into the source I cannot know how
this is handled in cxxtools::http::client. Will there be keep alive
after the initial request - or with the planned modified function after
connect, if realConnect was true - until the instance of Client is
destructed ? Not seeing any hint in the API would suggest that no keep
alive is done.
Another example: There is no close() function and the socket is not
exposed in the interface, so I assume that the Client class is thought
to be instantiated on the go, when needed, and not to be kept around for
reuse. But is this assumption correct ? A short comment on the class
level could easily shed light on the philosophy behind the code.
The http client uses always keep alive. It set up the connection when
needed and holds it as long as the class exists. When a request is
executed, it tries to use the existing connection. If it do not work any
more e.g. since the server was restarted, it creates a new connection.
This is all transparent to the user. The user do not need to care about
network connections.
It is currently not possible to suppress keep alive in the client and I
don't see any good reason not to use it.
The same is true for the rpc client classes.
There is actually a close() method in tcpsocket. cxxtools::TcpSocket is
derived from IODevice, which is derived from Selectable. And this has a
close() method, which is inherited.
If you agree, I will suggest comments for functions/classes (via github
pull request) as I progress and stumble upon questions I cannot solve
without digging deeper into the library.
Proposed skeleton of a comment for connect() :
/**
* connect() sets the host address and port for the socket used.
* By default no connection is opened. This is done upon the first request.
* Setting realConnect to true will actually connect. On failure to connect an
x exception will be thrown, but the host address and port will be kept.
* keepalive behaviour: (may be better to comment on class level)
* exceptions raised: ...
*/
One problem is, that there are multiple connect methods, which share the
same behavior. There for example:
void cxxtools::http::Cilent::connect(const cxxtools::net::AddrInfo&,
bool realConnect = false);
void cxxtools::http::Cilent::connect(const std::string& host,
unsigned short int port, bool realConnect = false);
void cxxtools::xmlrpc::HttpClient::connect(const net::AddrInfo&
addrinfo, const std::string& url, bool realConnect = false);
void cxxtools::xmlrpc::HttpClient::connect(const net::Uri& uri, bool
realConnect = false);
void cxxtools::xmlrpc::HttpClient::connect(const std::string& addr,
unsigned short port,
const std::string& url, bool realConnect = false);
void cxxtools::json::RpcClient::connect(const std::string& addr,
unsigned short port, bool realConnect = false);
void cxxtools::json::HttpClient::connect(const net::AddrInfo&
addrinfo, const std::string& url, bool realConnect = false);
void cxxtools::json::HttpClient::connect(const net::Uri& uri, bool
realConnect = false);
void cxxtools::json::HttpClient::connect(const std::string& addr,
unsigned short port,
const std::string& url, bool realConnect =
false);
void cxxtools::bin::RpcClient::connect(const std::string& addr,
unsigned short port, bool realConnect = false);
Not to forget the constructors of those classes since you can pass the
connection parameters there directly. How do we handle that? There is a
cross reference tag in doxygen (\see?). Maybe the behavior of connect
should be there somewhere (where?) and add cross references into all
connect methods and constructors.
There is a short comment about that in cxxtools/http/client.h already:
// Sets the host and port. No actual network connect is done unless
realConnect is set.
void connect(const net::AddrInfo& addrinfo, bool realConnect = false);
I already discussed with Olaf Radicke about doxygen comments. There is a
lack of those but as you can see, it is not that easy. Also even if
there are doxygen comments, it is not given, that people find them, as
you can see in the close() method.
Doxygen is fine but it is not the solution for all documentation. I've
written some howtos to give people a idea, how to use the functionality.
Olaf is writing a book, which is great also.
And yes please, add documentation and send me pull requests. That would
be great. It is good to see people, who start working instead of just
saying, that someone has to do something.
Tommi
------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk
_______________________________________________
Tntnet-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tntnet-general
