Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

The use of closure as associated value of the processBody case feels unusual #90

Open
hartbit opened this issue Nov 8, 2017 · 3 comments

Comments

@hartbit
Copy link

hartbit commented Nov 8, 2017

I've been looking through example code for the HTTP library and the use of a associated value closure for the HTTPBodyProcessing.processBody case feels kind of usual. I've never seen such a pattern used in the Standard Library. And if this library's future is to be integrated into the Standard Library or shipped with Swift, I think it would be worth looking into keeping as close as possible to the API style of the Standard Library.

Any ideas?

@helje5
Copy link
Contributor

helje5 commented Nov 8, 2017

I agree that the API isn't particularly nice (in fact I think it is pretty weird :-) ) But it does the job.

So what is the API you suggest @hartbit ?

@hartbit
Copy link
Author

hartbit commented Nov 8, 2017

I don't know if this is any better:

public protocol HTTPRequestHandling: class {
    func handle(request: HTTPRequest, response: HTTPResponseWriter, processBody: HTTPBodyHandler)
}

class EchoHandler: HTTPRequestHandling {
    func handle(request: HTTPRequest, response: HTTPResponseWrite, processBody: HTTPBodyHandler) {
        //Assume the router gave us the right request - at least for now
        response.writeHeader(status: .ok, headers: ["Transfer-Encoding": "chunked", "X-foo": "bar"])
        processBody { (chunk, stop) in
            // handle body
        }
    }
}

@helje5
Copy link
Contributor

helje5 commented Nov 8, 2017

And if the processBody closure is not set yet, it defaults to discard? Yeah, maybe. Looks a little nicer to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants