Passing status code to sendRequest handler.

[pocket7878]

Some API returning multiple status code, so client needs to know what status code was returned.

[ishkawa]

Actually, passing status code is important in some practical case, but I'm hesitating to add an argument to completion handler because there are many case that status code is not necessary. Personally, I think it would be better to keep current implementation of sendRequest and add new sendRequest with a handler that contains statusCode as below.

public class func sendRequest<T: Request>(request: T, URLSession: NSURLSession = defaultURLSession, handler: (Result<T.Response, NSError>) -> Void = {r in}) -> NSURLSessionDataTask?
public class func sendRequest<T: Request>(request: T, URLSession: NSURLSession = defaultURLSession, handler: (Result<T.Response, NSError>, Int?) -> Void = {r in}) -> NSURLSessionDataTask?

These methods can pass HTTP status code and keep backward compatibility.

[pocket7878]

Thank you for you reply.
Yes I think so too. And I was trying to adding backward compatibility function like this:

// send request and build response object
public class func sendRequest<T: Request>(request: T, URLSession: NSURLSession = defaultURLSession, handler: (Result<T.Response, NSError>) -> Void = {r in}) -> NSURLSessionDataTask? {
    return self.sendRequest(request, URLSession: URLSession, handler: { (r, s) in
        handler(r)
    })
 }

But it gives me and error 'Ambiguous use of 'sendRequest' when I trying APITests.
How can I fix it?(Sorry for my poor knowledge about swift 😢 )

[ishkawa]

Ah, Swift compiler can't determine type because response in closure can be inferred as both (Result, Int) and (Result). To avoid this error, caller have to specify type by annotation (response: Result), but this does not match convention of other APIs of APIKit.

Anyway, we have to make breaking change to pass status code to caller. There are some ways:

• Add statusCode to closure argument (this PR).
• Add wrapper object that contains result and statusCode.

class Response<T> {
    let statusCode: Int?
    let result: Result<T, NSError>
}

@pocket7878 @ikesyo How do you think?

[pocket7878]

Hm...
Some API returns custom response header, so soon it will be required to access more information about server response(not now...).
So I think:

• if we adding argument to current sendRequest's handler then It must be some object that represent server response for future extension.
• If we choose to wrapping result into Response object, then just adding new fields to Response.

I prefer former than later. Because it seperating concern.

[ikesyo]

Sorry for the late response. I was on vacation last week. 😅

• if we adding argument to current sendRequest's handler then It must be some object that represent server response for future extension.
• If we choose to wrapping result into Response object, then just adding new fields to Response.

I also prefer the former. How about just passing NSURLResponse or NSHTTPURLResponse?

[ishkawa]

OK, let's add NSHTTPURLResponse to arguments of completion handler. The next topic is order of arguments. IMO, Result should be the first argument because it is the most important argument of completion handler. As a result, new interface is as follows:

sendRequest(request) { result, response in
    switch result {
    case .Success: // ...
    case .Failure: // ...
    }
}

[ikesyo]

👍 for Result in the first place.

[pocket7878]

@ikesyo @ishkawa Thank you for your comment!! 😄
I just updated to pass HTTPURLResponse.

[ishkawa]

result, response?

[ishkawa]

👍 except for 1 note. Thank you for nice patch!

[ikesyo]

Style: An extra space.

[ikesyo]

I'm sorry for the nitpicks. 😅 Other than them, looks good! 👌

[pocket7878]

@ishkawa @ikesyo Thank you for checking code formats !

[ishkawa]

@ikesyo @pocket7878
My opinion is changed. So sorry for the late suggestion 🙇

All the information about response should be expressed in Response (typealias in Request).
If some value in HTTP response (e.g. status code) makes difference of the response object,
it should be handled in responseFromObject. And the same mechanism should also be added to responseErrorFromObject. That is, adding NSHTTPURLResponse to the arugments of responseFromObject and responseErrorFromObject might be better than adding it to that of closure in sendRequest.

Related: #38, #48

Sample code for pagination:

struct PaginatedResponse<T> {
    var results: Array<T>
    var nextPage: Int { get }
    var hasNext: Bool { get }

    init(results: Array<T>, URLResponse: NSHTTPURLResponse) {
        self.results = results
        self.nextPage = /* get nextPage from `Link` field of URLResponse */
        self.hasNext = /* get hasNext from `Link` field of URLResponse */
    }
}

struct SomePaginatedRequest: Request {
    typealias Response = PaginatedResponse<Some>

    var URLRequest: NSURLRequest? {
        return SomeAPI.URLRequest(
            method: .GET,
            path: "/paginated",
            parameters: ["page": page]
        )
    }

    let page: Int

    static func responseFromObject(object: AnyObject, URLResponse: NSHTTPURLResponse) -> Response? {
        var somes = [Some]()
        if let dictionaries = object as? [[String: AnyObject]] {
            for dictionary in dictionaries {
                if let some = Some(dictionary: dictionary) {
                    somes.append()
                }
            }
        }

        return PaginatedResponse(results: somes, URLResponse: URLResponse)
    }
}

let request = SomeAPI.SomePaginatedRequest(page: 1)

SomeAPI.sendRequest(request) { result in
    switch result {
    case .Success(let box):
        let response = box.value
        // response.results, response.hasNext and response.nextPage are available

    case .Failure:
        break
    }
}

Another example for 204, which indicates empty body:

class SomeRequest: Request {
    typealias Response = Some?

    var URLRequest: NSURLRequest? {
        return SomeAPI.URLRequest(
            method: .GET,
            path: "/some"
        )
    }

    class func responseFromObject(object: AnyObject, URLResponse: NSHTTPURLResponse) -> Response? {
        switch URLResponse.statusCode {
        case 204:
            // 204 indicates empty body
            return nil

        default:
            return Some(object: object)
        }
    }
}

[pocket7878]

@ishkawa Sorry for my late reply.
Currently I'm difficult to take time to care this Pull Request.
So maybe I had better to close this PR.

[ishkawa]

OK, I will close this pull request. Thank you for giving a chance to think about HTTP status code and closure design. The idea I mentioned above is now available on master branch. Please try master if you need it!