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

[Refactoring] Replace many of force unwrapping usages with optional binding and where clause #557

Merged
merged 1 commit into from
Jul 10, 2015

Conversation

ikesyo
Copy link
Contributor

@ikesyo ikesyo commented Jun 30, 2015

No description provided.

if downloadTaskDidFinishDownloadingToURL != nil {
let destination = downloadTaskDidFinishDownloadingToURL!(session, downloadTask, location)
if let handler = downloadTaskDidFinishDownloadingToURL {
let destination = handler(session, downloadTask, location)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all the places where you are replacing force unwrapping with optional binding, could you use the following format instead?

if let downloadTaskDidFinishDownloadingToURL = self. downloadTaskDidFinishDownloadingToURL {
    let destination = downloadTaskDidFinishDownloadingToURL(session, downloadTask, location)
}

We have a open task for replacing all property accesses with the self. prefix for clarity. This would be a good place to start that work on all these changes.

@cnoon
Copy link
Member

cnoon commented Jul 10, 2015

Thanks for this PR @ikesyo! I have just a single comment that will require a few changes throughout the PR. If you could get those changes made, I'll gladly merge it.

Thank you for taking the time to clean up the logic.

@cnoon cnoon added this to the 1.3.0 milestone Jul 10, 2015
@cnoon cnoon self-assigned this Jul 10, 2015
@ikesyo ikesyo force-pushed the optional-binding branch from bb9e0a4 to 9041e45 Compare July 10, 2015 15:32
@ikesyo
Copy link
Contributor Author

ikesyo commented Jul 10, 2015

@cnoon Thank you for the review! I've fixed the format what you pointed and rebased to master. Could you check it?

cnoon added a commit that referenced this pull request Jul 10, 2015
[PR #557] Refactoring force unwrapping usage with optional binding and where clauses.
@cnoon cnoon merged commit 603b21f into Alamofire:master Jul 10, 2015
@cnoon
Copy link
Member

cnoon commented Jul 10, 2015

Great work @ikesyo...thanks for the changes! 🍻

@ikesyo ikesyo deleted the optional-binding branch July 11, 2015 00:38
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

2 participants