Separate "Request" and "Task" APIs? about elwebservice HOT 2 CLOSED

👍 👍

I am very much in favor splitting up ServiceTask from Request and have been wanting to propose a change like this with the intention of introducing it into v4.

This is how I've envisioned the changes:

• Remove request building API from ServiceTask.
• Make Request's initializer public so that requests can be constructed independently of both WebService and ServiceTask
• Change the request API of WebService (ie .GET(path:) and its variants) to return a Request value that has been initialized with a URL relative to WebService's baseURL configuration instead of returning a ServiceTask.
• Add a method to WebService for creating a ServiceTask from a Request, something like: func serviceTask(request:) -> ServiceTask.

With the Request initializer made public, a Request struct can be directly created and mutated.

// Build a request independently of the ServiceTask chain
var request = Request(.POST, url: "http://httpbin.org/")
request.formParameters = ["foo": "bar"]

// Create ServiceTask from request
service
  .serviceTask(request: request).
  .response { data, response in

  }
  .resume()

Building a request off of WebService would give you a request that is based on the baseURL of WebService

// Build a request based on the `WebService` baseURL configuration
var request = service.POST("foo")
request.formParameters = ["foo": "bar"]

// Create ServiceTask from request
service
  .serviceTask(request: request).
  .response { data, response in

  }
  .resume()

The chaining methods could be moved to Request although I am in favor of eventually removing them entirely.

// Build a request independently of the ServiceTask chain

var request = service
                  .POST("foo")
                  .setFormParameters(["foo": "bar"])


// Create ServiceTask from request
service
  .serviceTask(request: request).
  .response { data, response in

  }
  .resume()

Of course this means that syntax like this will no longer be possible but this should be considered an improvement for the sake of clarity and safer code:

service
  .POST("foo")
  .setFormParameters(["foo": "bar"])
  .response { data, response in

  }
  .resume()

We could retain the chaining behavior from request-to-response if we added a method to Request that returns a ServiceTask, like your send() example:

service
 .POST("foo")
  .setFormParameters(["foo": "bar"])
  .send()
  .response { data, response in

  }
  .resume()

But reading this code is a little odd because send() isn't actually sending anything. The request is actually sent the first time resume() is called and I'd like to stick to the resume/cancel/suspend API in order to be consistent with NSURLSession and friends. A method like send() would be misleading next to the other lifecycle methods.

It is worth noting that the original API for building requests was not well-received and there was a lot of Slack discussion to change it that ended with the consensus favoring the chaining style for request building. Also worth noting is that this setter chaining style is called out as something that should be avoided in the best practices guide that we forked into Electrode-iOS.

Since this has been a debated subject in the past, I'd like to get some feedback from others before we make a decision on what the API should look like to separate ServiceTask from Request but I definitely want to see this happen for v4.

from elwebservice.

Glad to hear you feel the same way. There's one thing that concerns me about this pattern:

service
  .serviceTask(request: request)
  .response { data, response in /* snip */ }
  .resume()

Which is that there is no help from the compiler to make sure that you call resume(). If you don't call it, the task just sits there waiting.

Thinking about it, I'm not sure that resume() is even useful. AFAICT, it's purpose is to allow you to configure the request before sending it. But with the new API, that will be done by mutating the request before passing it to serviceTask().

from elwebservice.