Hi Peter,
thanks for comments!
Http::Header and its derived classes should implement a Value Object, so we should provide operator==(). The operator()== implementation compating the raw representation of the Header objects should actually work for all Header variants.
Ok, I’ve implemented it. You can see it in my rev. 159 changeset.
get() will most likely not always be able to complete synchronously. I don’t think get() would ever fail actually, not synchronously at least, so void is OK. If the Transport gets disconnected, the session would send sessionClosed(). If the received response could not be correctly parsed, sessionClosed() could be sent as well. At first I would not try to differentiate between the different kinds of errors - the user (code) would generally only care to know whether the get() worked at all.
I’ve added a session closing. If there is a problem during response handling, both sessions will be closed (HttpRequestSession and TrackerRequestSession). I’m not sure whether I should add an abort() member function, though.
We can never be 100% certain whether the request was actually sent and not at all sure whether it has been received. The Http::RequestHeader object could be buffered in HttpRequestSession, but it could also be buffered in Transport, or it could be buffered by the OS, or some router on the Internet. Again, I don’t think that the user of the class would ever need to know much of the error details - it would generally care only of the end result - response was received or not.
Ok, I agree!
I hope you also start to feel the power of testing with Mock Objects - you can express your design ideas in code without even writing implementation!
Yes, it’s great ;).
What I’d suggest you though (that’s the TDD philosophy) is to write a single test that fails, make it pass, then refactor both the test and the implementation if needed. The test and the implementation would very often be “stable” only after a few iterations. Unless there is a compelling reason to do so (you need to get a better overview of the problem, or you are certain you can do it right), do not write too many tests at once! Considered that as a guideline rather then a general rule though! Your experience will be able to tell you what is appropriate in which cases.
Ok, thanks for tips - I’ll try to follow them!
I rewrote (changeset) the implementation of the TrackerRequestSession according to the tests/todos. I’ve also added a testing scenario for verifying response handling, as you suggested in one \todo. There are now various scenarios like recieving HTTP 200 OK, but invalid response, recieving HTTP 400 etc. Session closing is handled properly (two tests still fail, though, because TrackerResponse/TrackerResponseParser implementation is not done).
You can have a look on the implementation, I hope I’ve managed to implement it successfully. Nevertheless, comments are welcomed!
I’ll now implement the TrackerResponse/TrackerResponseParser classes, so the TrackerRequestSession (tests) could work properly. I’ll use a similar “pattern” as I’ve used in the Torrent/TorrentParser classes.
Have a nice day,
Petr
