Return to Answer

I don't like passing bools as parameters. I really dislike a function like your SimpleSocketStreamBuffer constructor that take multiple bools. You need to do a fair amount of looking to be sure how:

foo x("www.google.com", false, true, bar);

differs from:

 foo x("www.google.com", true, false, bar);

(...and the same for the other variations as well). I'd strongly prefer to create an enumeration and have the parameters of that type.

std::function

I don't like passing a std::function as a parameter either. I'd prefer to see a template parameter to specify the function type, and then use an std::function only internally for storage. With those, the constructor looks something like this:

enum class curl_type { easy, hard };
enum class dl_strategy { lazy, greedy };
template <class func>
SimpleSocketStreamBuffer(std::string const& url, 
 curl_type ct, 
 dl_strategy download, 
 func markStreamBad)

Then we modify the remainder to suit:

, preDownload(download == dl_strategy::greedy)

and:

if (ct == curl_type::easy)
 // ...

Of course, if you're going to do this, you want to do it throughout (e.g., to the stream definition, not just the stream buffer definition).

Using these, defining an object looks more like this:

foo x("www.google.com", curl_type::easy, dl_strategy::greedy, bar);

...which strikes me as quite a bit more self-explanatory.

userdata pointer

In writeFunc and headerFunc, it would probably be best to verify that userdata isn't a null pointer before using it. Once you've verified that it's non-null, I think I'd prefer to define owner as a reference instead of a pointer:

SimpleSocketStreamBuffer & owner = *reinterpret_cast<SimpleSocketStreamBuffer*>(userdata);

This prevents accidentally re-assigning owner to point anywhere else, and (arguably) simplifies the rest of the code a bit by letting you use . instead of -> when you're dealing with the owner object.

buffer manipulation

I also don't particularly like the code you used to add data to the end of the buffer:

owner->buffer.resize(oldSize + bytes);
std::copy(ptr, ptr + bytes, &owner->buffer[oldSize]);

I think I'd prefer to just insert the data in a single step:

owner->buffer.insert(owner->buffer.end(), ptr, ptr + bytes);

lambda syntax

There's one other change I'd consider, but I'm a bit less certain about whether I'd actually make it. A lambda that takes no parameters doesn't need to include empty parens. Using this (and taking the preceding changes into account) the ctor for IThorSimpleStream can end up looking like this:

IThorSimpleStream(std::string const& url, dl_strategy s = dl_strategy::lazy)
 : std::istream(NULL)
 , buffer(url, curl_type::easy, s, [this]{this->setstate(std::ios::badbit); })
 {
 std::istream::rdbuf(&buffer);
 }

Particularly given the degree to which people are undoubtedly accustomed to using empty parens when defining functions, this could lead to some confusion. I suspect when people are more accustomed to lambda syntax, we'll probably view the empty parens are kind of foolish looking, but for now it may be better to leave them there.

lang-cpp