-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix Swift 3.1 warnings. #73
Conversation
Vinyl/Response.swift
Outdated
@@ -60,9 +60,9 @@ func ==(lhs: Response, rhs: Response) -> Bool { | |||
extension Response: Hashable { | |||
|
|||
var hashValue: Int { | |||
let body = self.body == nil ? "\(self.body)" : "" | |||
let error = self.error == nil ? "\(self.error)" : "" | |||
let body = self.body == nil ? "" : "\(self.body!)" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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())"
There was a problem hiding this comment.
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)" } ?? ""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
No description provided.