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

ResponseObjectSerializable's creation method should return a (T?, NSError?) tuple #230

Closed
jqsilver opened this issue Nov 22, 2014 · 2 comments

Comments

@jqsilver
Copy link

I have mixed feelings about failable initializers, but I also deal a lot with missing or invalid data in my json responses. The example you give of an object that implements ResponseObjectSerializable will crash if the "name" value is not a string.

final class User: ResponseObjectSerializable {
  let username: String
  let name: String

  required init(response: NSHTTPURLResponse, representation: AnyObject) {
    self.username = response.URL!.lastPathComponent
    self.name = representation.valueForKeyPath("name") as String
  }
}

I'd propose something like this instead, to match the other serialization method signatures:

protocol ResponseObjectSerializable {
    class func buildFromResponse(response: NSURLResponse, representation: AnyObject) -> (Self?, NSError?)
}
@mattt
Copy link
Sponsor Contributor

mattt commented Nov 23, 2014

At the time of writing, Swift did not yet have failable initializers, which is definitely something that would be desirable when interacting with unknown and potentially untrusted data from an external web service. b5d6f58 makes this change to add such a consideration.

@mattt mattt closed this as completed Nov 23, 2014
fabiopelosin added a commit to fabiopelosin/Alamofire that referenced this issue Nov 27, 2014
…podspec

* 'master' of https://github.com/Alamofire/Alamofire:
  [Issue Alamofire#230] Making response object example initializer failable
  Fixing optional textLabel property on cells
  Adding missing forced unwrap in test
  [Issue Alamofire#226] Fixing optional cookie entry in cURL output
  Fixing possible exception when force unwrapping optional header properties
@krzd
Copy link

krzd commented Dec 22, 2014

The code after your change @mattt does not compile:
completionHandler(request, response, object, error)
complains that AnyObject is not T, therefore I would need to change it to:
completionHandler(request, response, object as T?, error)

But even then your example is not there yet, because if there is a missing value it will still just crash instead of failing and returning nil with the message:
fatal error: unexpectedly found nil while unwrapping an Optional value
This happens when doing something liek this and the value is not present:
self.name = representation.valueForKeyPath("name") as String

What would be the best way of going about it?

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

No branches or pull requests

3 participants