Code review comment for lp:~marcustomlinson/unity-scopes-api/lp-1267917-2

This is a bit of a tricky situation. Consider the requirements:

1. HttpClientQt.h should not contain Qt code that requires any more than forward declarations (this is to avoid anything outside of this class having to link to Qt).

2. A call to HttpClientQt::get needs to return a handle to the future result and asynchronously execute the request.

3. QSocketNotifier (in QNetworkAccessManager) can only be used with threads started with QThread.

4. The manager_ and reply_ Qt members (regardless of who their parent class is) need to be created and destroyed in the same QThread.

In order to satisfy req.1, we include the HttpClientThread class in the HttpClientQt cpp file. This class can then contain all the Qt specific stuff like signals/slots.

req.2 is performed by spawning a C++11 worker thread on request which eventually assigns a promise with the result. This thread then uses an HttpClientQtThread object to execute the query (yes, HttpClientQtThread is not really used as a worker thread as you pointed out)

Considering we already have a C++11 worker thread per request, does the helper Qt class really need to be a QThread? Well yes. I tried this class as just a QObject with synchronous methods but ended up with errors on new-ing QNetworkAccessManager (hence req.3).

Naturally then, as we now have a QThread available, we can use this thread to create, use and destroy the manager_ and reply_ HttpClientQtThread members (req.4). Lastly, because we need to access the reply_ object after it is assigned but before it is destroyed, we need to suspend the QThread until HttpClientQtThread signals it to exit (on destruction).

I hope this makes some sense as to why the code is structured this way (By no means am I saying that this is the only possible solution).