HTTP Stream Part 2 A stream that opens an HTTP GET and then acts like a normal C++ istream

The review I asked for yesterday was for a header only library.

Stream that opens an HTTP GET and then acts like a normal C++ istream

This has some limitations as it downloads all the data as part of the constructor. This can lead to delays during start up if the file is large (usually during REST communications the size is small so the header only version is sufficient).

But if you want larger files and don't want a delay here is a version that downloads the data in the background asynchronously. If the stream buffer underflows the next block (which has been accumulated in the background) is quickly swapped into the buffer and the main thread can continue processing.

The disadvantage is that this is no longer a header only library and you will need to link against the library.

https://github.com/Loki-Astari/ThorsStream
ThorsStream.h

```
#ifndef THORSANVIL_STREAM_THOR_STREAM_H
#define THORSANVIL_STREAM_THOR_STREAM_H

// Note "ThorsSimpleStream.h" is up fro review in a seprate question
// https://codereview.stackexchange.com/questions/38402/http-stream-a-stream-that-opens-an-http-get-and-then-acts-like-a-normal-c-istr
#include "ThorsSimpleStream.h"
#include
#include

namespace ThorsAnvil
{
namespace Stream
{

class IThorStream: public std::istream
{
// Class to handle the down-loading of all http connections in the background.
class ThorStreamManager
{
public:
// Everybody will use the same instance
static ThorStreamManager& defaultManager()
{
static ThorStreamManager defaultManager;
return defaultManager;
}

ThorStreamManager()
: finished(false)
, multi(curl_multi_init())
, streamThread(&ThorStreamManager::eventLoop, this, multi != NULL)
{
if (multi == NULL)
{ throw std::runtime_error("Failed to startup");
}
}

#include "ThorsSimpleStream.h"

There's no ThorsSimpleStream identifier used in the header: instead, "ThorsSimpleStream.h" declares class IThorSimpleStream.

I prefer it if the filename of the header matches the name of identifier which it declares.

IThorSimpleStream declares friend class IThorStream.

I prefer it if friend classes are defined in the same header file (this suggestion comes from Lakos, summarized as "no long-distance friendship").

SimpleSocketStreamBuffer seems to be a private member of IThorStream and IThorSimpleStream. Instead of declaring friendship between IThorStream and IThorSimpleStream, perhaps you could simply define SimpleSocketStreamBuffer in its own header file.

class IThorStream

I'm used to I being used for designate 'interfaces': or pure abstract classes in C++. I don't see why you're prefixing your class names with I.

You could move more out of the header and into the CPP file: the definition/implementation of SocketStreamBuffer methods, and the whole ThorStreamManager class declaration.

Users of your header file would then have less to read, and fewer headers such as ` included into their code.

You could hide the whole SocketStreamBuffer declaration too, using the 'cheshire cat' aka 'pimpl' idiom (i.e. by making it an opaque pointer).

lock.unlock();
           count = pselect(max_fd+1, &read, &writ, &exec, &timeoutSpec, NULL);
           lock.lock();

Might you want to do if (finished) break; after re-acquiring the lock, in case the system is shutting down / waiting to shutdown?


catch(std::exception const& e)

This is outside the while loop so the thread will die so the ThorStreamManager is dead. You might like to move that into the while loop, or re-create the ThorStreamManager` in case there's a next request.

(I haven't learned the CURL API so can't review your eventLoop in any detail.)

StackExchange Code Review Q#38455, answer score: 3