Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature - Response Generic Type #792

Merged
merged 5 commits into from Sep 22, 2015

Conversation

cnoon
Copy link
Member

@cnoon cnoon commented Sep 19, 2015

This PR creates a generic Response type to help simplify response serialization. Instead of returning a tuple with four different values, response serializers now return a single generic Response object. This makes it MUCH easier to use response serializers, and will help users avoid making mistakes such as incorrectly naming the tuple parameters.

I've seen MANY cases of people swapping the request, response, data and result parameters in the tuple. They constantly get misused with tends to lead to a large amount of confusion. Additionally, it can cause strange compiler errors that are quite difficult to debug. This PR attempts to address these issues by replacing the tuple with a generic Result type with a generic Response struct.

Old Usage

Alamofire.request(.GET, "http://httpbin.org/get", parameters: ["foo": "bar"])
    .response { request, response, data, result in
        print(request)
        print(response)
        print(data)
        debugPrint(result)
    }

New Usage

Alamofire.request(.GET, "http://httpbin.org/get", parameters: ["foo": "bar"])
    .response { response in
        debugPrint(response)

        // or

        print(response.request)
        print(response.response)
        print(response.data)
        debugPrint(response.result)
    }

This should really help alleviate misuse and should simplify response serialization usage with the concrete struct.

@cnoon cnoon added this to the 3.0.0 milestone Sep 19, 2015
@cnoon cnoon self-assigned this Sep 19, 2015
@cnoon
Copy link
Member Author

cnoon commented Sep 19, 2015

cc @kcharwood and @kylef

@cristeahub
Copy link

I would suggest adding an updated README along with this to help avoid confusion.

@cnoon
Copy link
Member Author

cnoon commented Sep 20, 2015

Absolutely @cristeahub. I'd rework all the README code samples and put together another migration guide.

…eneric_type

# Conflicts:
#	Source/ResponseSerialization.swift
@marcelofabri
Copy link

Another advantage of this is that if another parameter is added (or removed), people won't have to change every response call.

@tobiasoleary
Copy link

@macelofabri agreed! Recently upgraded a project to Swift 2.0. Even with regular expressions updating the code for the latest version of Alamofire was a pain.

@kcharwood
Copy link
Contributor

I like this change as well. Will definitely make it easier to modify the object in the future if we need too, although it will be one more round of upgrades to every call site @tobiasoleary 🍻

cnoon added a commit that referenced this pull request Sep 22, 2015
@cnoon cnoon merged commit 00a0130 into feature/double_generic_results Sep 22, 2015
@cnoon cnoon deleted the feature/response_generic_type branch September 22, 2015 03:24
@kylef
Copy link
Contributor

kylef commented Sep 22, 2015

👍

@cristeahub
Copy link

Quick question here.

The Alamofire.response function doesnt use this system here as seen in the documentation and here. Was this only intended for responseJSON and similar?

@cnoon
Copy link
Member Author

cnoon commented Nov 10, 2015

It was definitely intended @cristeahub. The response method simply forwards all the data collected from the SessionDelegate without any additional processing. The others are all customized to process the data in a specific way.

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

Successfully merging this pull request may close these issues.

None yet

6 participants