Skip to content

Fix Swift 3.1 warnings. - #73

Merged
dmcrodrigues merged 3 commits into
masterfrom
swift31-fixes
Apr 20, 2017
Merged

dmcrodrigues merged 3 commits into
masterfrom
swift31-fixes

Conversation

@mluisbrown

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread Vinyl/Response.swift Outdated
var hashValue: Int {
let body = self.body == nil ? "\(self.body)" : ""
let error = self.error == nil ? "\(self.error)" : ""
let body = self.body == nil ? "" : "\(self.body!)"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, no, don't use force unwrapping.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's force unwrapping in a context where self.body is guaranteed to be not nil. What alternative would you suggest? Incidentally, the previous code was broken as the logic was the opposite of what was intended.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, because self.body is a Data and "" is a String, and we want the result to be a String. In the case of the body this compiles:

let body = "\(self.body ?? Data())"
But the equivalent for the Error does not...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@RuiAAPeres would this work for you?

let body = "\(self.body ?? Data())"
let error = "\(self.error ?? NSError())"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mluisbrown can you please change to this?

let body = self.body.map { "\($0)" } ?? ""
let error = self.error.map { "\($0)" } ?? ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@dmcrodrigues
dmcrodrigues merged commit 4479f4d into master Apr 20, 2017
@dmcrodrigues
dmcrodrigues deleted the swift31-fixes branch April 20, 2017 22:28
@RuiAAPeres

Copy link
Copy Markdown
Member

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

Labels

None yet

3 participants