cavaliergopher / grab

A download manager package for Go
BSD 3-Clause "New" or "Revised" License
1.38k stars 151 forks source link

Add DoChannel method #13

Closed oliverpool closed 7 years ago

oliverpool commented 7 years ago

Fix #12.

I simply made a copy/paste of DoBatch. Now my DoBatch looks like this:

func (c *Client) DoBatch(workers int, reqs ...*Request) <-chan *Response {
    // TODO: enable cancelling of batch jobs

    // start work queue
    producer := make(chan *Request, 0)
    go func() {
        // feed queue
        for i := 0; i < len(reqs); i++ {
            producer <- reqs[i]
        }
        close(producer)
    }()

    // default one worker per request
    if workers == 0 {
        workers = len(reqs)
    }

    return c.DoChannel(workers, producer)
}

It is the callers responsibility to close the reqs channel.

oliverpool commented 7 years ago

Since DoChannel is called by DoBatch, testing DoBatch automatically tests DoChannel ;-)

oliverpool commented 7 years ago

One issue is that the workers will block if the responses channel is full (this occurs for batch requests as well).

A possibility would be to pass the receiving channel as argument (as recommended by https://inconshreveable.com/07-08-2014/principles-of-designing-go-apis-with-channels/) or at least document this behavior.

cavaliercoder commented 7 years ago

Specifically, breaking principle 4?