I like linear flows. With Cocoa calls, I request, check for failure, handle errors, and move on. The step-by-step results are easy to follow, to modify, and to understand many months later.
Quick note: 1.2 doesn’t seem to like the NSErrorPointer pattern I use in this post. You can easily replace with inout error : NSError?
NSString *StringFromURL(NSURL *url, NSError **error)
{
// Request data
NSData *data = [NSData dataWithContentsOfURL:url options:0 error:error];
if (!data) return nil;
// Convert the data to a UTF-8 string
NSString *string = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
if (!string) {
if (error)
*error = BuildError(1, @"Unable to build string from data");
return nil;
}
// Return the string
return string;
}
The standard Swift equivalent for these checks uses if-let constructs. If-let provisionally assigns a variable to the Some enumeration case. When the optional can be unwrapped, it’s assigned to a local non-optional variable (here, data and string) and used. The if-let else clause handles errors. Unfortunately, the success case tends to be buried in a pyramid of code extending above and below it. Here, the return string success appears smack in the middle of this function.
func StringFromURL(url : NSURL, error errorPointer : NSErrorPointer) -> (NSString?) {
var error : NSError?
if let data = NSData(contentsOfURL:url, options: NSDataReadingOptions(rawValue:0), error: &error) {
if let string = NSString(data: data, encoding: NSUTF8StringEncoding) {
return string
}
if errorPointer != nil {
errorPointer.memory = BuildError(1, "Unable to build string from data")
}
return nil
}
if errorPointer != nil {
errorPointer.memory = error
}
return nil
}
The following adaptation avoids the if-let pyramid, testing for nil cases instead of Some. It is wordier. It uses more variable declarations. But I rather like the clarity and readability and that the success case appears at the very bottom. I’d imagine this would be far easier to tweak and maintain especially if there are additional steps added at any future time.
func StringFromURL(url : NSURL, error errorPointer : NSErrorPointer) -> (NSString?) {
var error : NSError?
// Fetch data. Blocking.
let data_ = NSData(contentsOfURL:url, options: NSDataReadingOptions(rawValue:0), error: &error)
if (data_ == nil) {
if errorPointer != nil {
errorPointer.memory = error
}
return nil
}
let data = data_!
// Convert to string.
let string_ = NSString(data: data, encoding: NSUTF8StringEncoding)
if (string_ == nil) {
if errorPointer != nil {
errorPointer.memory = BuildError(1, "Unable to build string from data")
}
return nil
}
let string = string_!
return string
}
What do you think? Heresy? Approval? Drop a comment.
11 Comments
if (string == nil where errorPointer != nil) {
errorPointer.memory = BuildError(1, “Unable to build string from data”)
return nil
}
It needs to return nil in both cases (passed error pointer and passed nil), so the where errorPointer doesn’t really help here
An Apple engineer weighed in on one aspect of this here: https://twitter.com/_jackhl/status/564871591796822016
Here’s hoping for a thread-safe “if not let” in the future.
Which property are we talking about here? I’m confused
Just a potential gotcha when dealing with variables that can be nil’d out by another thread. For completely local scope it shouldn’t be an issue.
IMHO this is how your example should look like
func StringFromURL(url : NSURL) -> (NSString?, NSError?) {
var error : NSError?
if let data = NSData(contentsOfURL:url, options: NSDataReadingOptions(rawValue:0), error: &error),
let string = NSString(data: data, encoding: NSUTF8StringEncoding)
{
return (string, error)
}
error = (error == nil) ? BuildError(1, “Unable to build string from data”) : error
return (nil, error)
}
If you are interested in both string and potential error it is better to use a tuple, on which the client can pattern match.
Having errorPointer as parameter in a new API makes no sense. This also simplifies the flow a lot.
In success case you return the string and the potential error.
In failure case you return nil as a string and given error or your own custom error if the error was not given.
theoreticaly you could simplify
error = (error == nil) ? BuildError(1, “Unable to build string from data”) : error
return (nil, error)
into:
return (nil, error ?? BuildError(1, “Unable to build string from data”))
practically you have to check if ?? operator will work on NSError. If not you can write your own implementation of ?? operator for NSError
[…] the early return on errors pattern might not turn out to idiomatic Swift. So far, I find it more readable than the monstrous […]
Why would anyone condemn this as heresy? I have adopted guard clauses because I think they make code so much easier to follow. When the happy path equals the path until the end of a method, reading code becomes so much easier, with a lot less jumping back and forth.
That said, I find it very difficult to do in Swift.
You say the method is wordier; right. But it’s also harder to split up. Being trained to make methods as short as possible in Ruby, I often struggle to extract sub-methods. There’s not much you could do in your example either.
func StringFromURL(url : NSURL, error errorPointer : NSErrorPointer) -> (NSString?) {fetchData(url, error: errorPointer).map { data in
stringFromData(data, error: errorPointer) {
}
}
if (data == nil) {
return nil
}
let string = stringFromData(data!, error: errorPointer)
if (string == nil) {
return nil
}
return string
}
In Objective-C, I’d have stopped here. But Swift has so many niceties which I still have to get right that I think in some cases it may be great to end up with something like this:
func StringFromURL(url : NSURL) -> Result<String> {return fetchData(url).transform { stringFromData($0) }.transform { stringFromNSString($0) }
}
I put together a blog post for this to elaborate a bit. I hope you don’t mind me posting the link. I’d love to get your opinion on this stuff: http://christiantietze.de/posts/2015/02/beyond-guard-clauses/
I’ve settled on:
let data = NSData(contentsOfURL:url, options: NSDataReadingOptions(rawValue:0), error: &error)!
if (data == nil) {
// handle error
}
// use data without extra !s in my code
That lets me skip the extra line to turn “data_” into “data”, which I think is too wordy. The only scary part is if I forget the ‘data == nil’ check, but, hey, then the app will just throw an exception and I’ll fix it.
-Wil Shipley
How about using a return variable to keep the original order but streamline a bit?
func StringFromURL(url : NSURL, error errorPointer : NSErrorPointer) -> (NSString?) {
var error : NSError?
var ret: NSString? = nil
if let data = NSData(contentsOfURL:url, options: NSDataReadingOptions(rawValue:0), error: &error) {
if let string = NSString(data: data, encoding: NSUTF8StringEncoding) {
ret = string
}
if errorPointer != nil {
errorPointer.memory = BuildError(1, “Unable to build string from data”)
}
}
else if errorPointer != nil {
errorPointer.memory = error
}
return ret
}