(PCP-410) Expose the connection timings #185
Conversation
parisiale
force-pushed
the
PCP-410
branch
2 times, most recently
from
2daebe4 to
f51796d
Compare
lib/src/connector/connector.cc
Outdated
| ConnectionTimings Connector::getConnectionTimings() const | ||
| { | ||
| return (connection_ptr_ == nullptr | ||
| ? ConnectionTimings() |
There was a problem hiding this comment.
I wonder where this ConnectionTimings() object is created - on the stack or heap?
There was a problem hiding this comment.
Temporary storage depends on context, if I'm correct. More here: http://www.learncpp.com/cpp-tutorial/79-the-stack-and-the-heap/
There was a problem hiding this comment.
It'll be stack, and copy elided to the caller.
|
LGTM |
| class LIBCPP_PCP_CLIENT_EXPORT Connection { | ||
| public: | ||
| /// To keep track of WebSocket timings | ||
| ConnectionTimings timings; |
There was a problem hiding this comment.
Why did you make this public?
There was a problem hiding this comment.
Since the new AssociationTimings will be stored in the Connector, it makes more sense to me to allow Connector accessing directly the ConnectionTimings instead of having a copy from a Connection getter call. This is done considering that the actual user interface is provided by Connector, that does not expose the Connection instance.
No strong opinion here, I'm happy to make it private again if you think it's the case.
|
LGTM |
Adding reset method to ConnectionTimings. Making public the ConnectionTimings member (timings) of Connection. Adding ConnectionTimings getters to Connector. Adding new unit tests.
3 participants
No description provided.