etc.curl: Formal review begin

[Andrei Alexandrescu]

On 8/24/11 9:35 AM, Walter Bright wrote:
> On 8/24/2011 2:18 AM, Johannes Pfau wrote:
>> Walter Bright wrote:
>>> On 8/17/2011 4:12 PM, David Nadlinger wrote:
>>>> the etc.curl module by Jonas Drewsen is at the front of the review
>>>> queue.
>>>
>>> Thanks for doing this. I preface my remarks with saying I have never
>>> done http: programming and know little about it.
>>>
>>> 1. The first, most obvious thing I'd like to do is present a URL and
>>> get a string back that is the contents of that web page:
>>>
>>> string contents = getURL("http://www.digitalmars.com");
>>>
>>
>> That's the first example in the API docs:
>>
>> // Simple GET with connect timeout of 10 seconds
>> Http.get("http://www.google.com").connectTimeout(dur!"seconds"(10)).toString();

I find this flow counterintuitive. It suggests that first a GET is 
issued and then the timeout is set. It takes a bit of getting used to 
things to realize that get() is actually preparing things and that the 
real work is done in toString.

> Ok, but 1. there's no indication in the doc that that's what it does
> because 1. it is an incomplete fragment and 2. a simpler interface (like
> what I proposed) would likely be better.
>
>
>> or, if you don't care about timeouts:
>> string contents = Http.get("http://www.google.com").toString();
>
> This example needs to be first and needs an explanation. Timeouts, error
> recovery, progress indicators, etc., should all be brought up later.

I'll note there's an annoying redundancy: the protocol appears twice, 
once in the URL and again in the type Http. That means if one needs to 
fetch from an URL transparently they'd have to go through e.g.:

string url = readln();
string contents;
if (url.startsWith("http://")) {
   contents = Http.get(url).toString();
} else if (url.startsWith("ftp://")) {
   contents = Ftp.get(url).toString();
} else {
   enforce(false, "Unrecognized protocol.");
}

Probably this simple level of abstraction should be provided by the 
library (with the added advantage that client code will automatically 
work with new protocols as the library includes them).

I'll be back with a thorough review.


Andrei