Re: [boost] [ann] Urdl - a library for downloading web content

Subject: Re: [boost] [ann] Urdl - a library for downloading web content
From: Christopher Kohlhoff (chris_at_[hidden])
Date: 2009-06-18 08:05:04

Jose wrote:
> * The #1 feature should be supporting http 1.1 well (also https). Many
> libaries provide a http 1.0 implementation but fail short of
> supporting the many options http provides (I know this is a huge
> undertaking!) . At this point I don't see the value of supporting file
> (or planning to support ftp).

Sometimes I need to use them :) Seriously though, the power of working 
with URLs is their polymorphism, so the more protocols the better IMHO.

> In this case, I don't understand the
> value of future support for runtime polymorphism (can you explain how
> user-supplied backends would work ?)

I'm thinking an option to register a factory function, e.g. something like:

   read_stream_impl_base* make_impl(const url& u) { ... }
   ...
   my_read_stream.set_option(implementation_factory(make_impl));

> * It would be great to clarify why you based the design on a buffered
> stream (below are my perceived pros/cons):
> Pros
> - easy to add support for new protocols with read_until
> - easier header parsing
> Cons
> - increased implementation complexitiy with istreambuf
> - Maybe small performance penalty

I'm not sure which buffered stream you're referring to here. Can you 
clarify?

> * In detail/http_read_stream.hpp
> 
> open_coro chains the whole sequence from connecting to
> request/response. I think it would be better to split it in two
> coroutines (one for opening the connection and another for sending a
> request and getting a reply). This would make it easier to later
> implement keep-alive.

That's what the connect/async_connect functions are for. I will revisit 
that division when adding connection pooling.

> In open_coro you don't use Stream as a template parameter. Is this
> because Stream is a reference and you don't need to enforce any
> concept ? (I am trying to learn from your great coding practices :) )

It's already a template parameter on the enclosing class, so isn't 
needed on the nested open_coro class.

Cheers,
Chris