Hi Peter,
I got a little confused by PeersManager at first - I though you were working on the SessionsManager. If the DHT nodes are also called Trackers, then I’d support your suggestion to rename the class back to TrackerManager.
In the specs (1, 2) there is written that “A peer implementing this protocol becomes a “tracker” and stores lists of other nodes/peers which can be used to locate new peers.”, so even if the DHT is “trackerless” by it’s meaning, there are “trackers” nonetheless. So I’ll rename it back to TrackersManager. The PeersManager name originated from the idea of the function of the class - getting peers info from various locations (tracker, DHT).
Next, my idea is that the PeersRequestSession class will be the interface for all request sessions (tracker, DHT) and there will be a class that implements this interface, for example PeersRequestSessionTracker (not sure about the name, though) which will internally use TrackerRequestSession, so we won’t need to modify it (and tests). Or do you think that we should derive TrackerRequestSession from the PeersRequestSession directly without using a mediator (PeersRequestSessionTracker)? I don’t mean the design pattern, but just a mediator.
You have created this PeersRequestSessionPtr because the factory is not only passing a pointer but it should be passing the ownership of the object too. Maybe you tried out with auto_ptr but got compilation problems?
There were two problems with auto_ptr so I couldn’t use it:
- auto_ptr destroys it’s object in it’s dtor, but since I have only one PeersRequestSessionMock (for all sessions), I can’t destroy it at the end of PeersManagerImpl::start member function, so that is why I created a PeersRequestSessionPtr derived class (PeersRequestMockedSessionPtr) which doesn’t destroy the object pointed by the pointer. Summary: I must destroy PeersRequestSession instances (real), but not PeersRequestSessionMock instances (faked). Possible solution MAY be (I’m not sure) creating an extra PeersRequestSessionMock for each session and call requestSessionMock.release() instead of requestSessionMock.get().
- auto_ptr doesn’t have comparison operators, so it won’t compile since CalitkoMocks needs all objects to have the
operator==operator. I tried to create my own checker (as in the GenericSessionTest.cpp) and also my ownbool operator== (const auto_ptr &, const auto_ptr &), but none of these worked. This took me about an hour to figure out why it won’t compile, because everything seemed to be right (types), but there was always only a longerror: no matching function for call to ...error produced by the CalitkoMocks :).
I think that these were the main problems why I decided to create my own wrapper for storing PeersRequestSession pointer. This solution works, but there are actually needed two operator new calls - one for the session itself and one for the wrapper, so there definitely should be a simpler solution.
Have a nice day,
Petr
