2
\$\begingroup\$

I'm drowning in Swift optionals and error handling syntax. I'm just converting a text file into an array of Strings. One would think that this could be a simple two or three liner but by the time I got it to work, I ended up with the following mess:

enum ImportError: ErrorType {
    case FileNotFound
    case CouldntGetArray
}

private func getLineArrayFromFile(fileName: String) throws -> Array<String> {

    let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)
    if path == nil {
        throw ImportError.FileNotFound
    }

    var lineArray: Array<String>?
    do {

        let content = try String(contentsOfFile: path!, encoding: NSUTF8StringEncoding)
        lineArray = content.componentsSeparatedByString("\n")

    }catch{
        throw ImportError.CouldntGetArray
    }

    return lineArray!
}

I actually don't really care about the using ErrorType enum, but I wanted to play around with the new Swift Error Handling syntax.

I thought I understood optionals before, but they were giving me a headache when combined with the do-try-catch statement. I also didn't know if I should return an Array or an Optional Array.

What are the best practices for a situation like this?

  • Error handling
  • Treatment of optionals
  • Code brevity/readability
\$\endgroup\$
4
\$\begingroup\$
func getLineArrayFromFile(fileName: String) throws -> Array<String>
  • The function does not get lines from an arbitrary file, but from a resource file.
  • The "get" prefix is usually not used in Objective-C or Swift.
  • Array<String> can be shortened to [String].

So my suggestion for the function name would be

func linesFromResource(fileName: String) throws -> [String]
let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)
if path == nil {
    throw ImportError.FileNotFound
}
  • As you are obviously using Swift 2, this can be simplified with the guard statement. Note that path is no longer an optional.
  • There are pre-defined NSError codes which can be used here instead of defining your own. This also gives better error descriptions for free.

Example:

guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
    throw NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError, userInfo: [ NSFilePathErrorKey : fileName ])
}
do {
    let content = try String(contentsOfFile: path!, encoding: NSUTF8StringEncoding)
    lineArray = content.componentsSeparatedByString("\n")
} catch {
    throw ImportError.CouldntGetArray
}

You are catching the error and throw your own error code in the failure case, so the actual error information is lost. Better just call the try String(..) and let an error propagate to the caller of your function:

let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)

Again, this gives better error descriptions for free.

So the complete method would now look like this (no optionals anymore, no forced unwrapping with !):

func linesFromResource(fileName: String) throws -> [String] {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        throw NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError, userInfo: [ NSFilePathErrorKey : fileName ])
    }
    let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
    return content.componentsSeparatedByString("\n")
}

And a typical usage would be

do {
    let lines = try linesFromResource("file.txt")
    print(lines)
} catch let error as NSError {
    print(error.localizedDescription)
} catch let error {
    print(error)
}

The reason for the final catch let error is that it is required that the catch statements are exhaustive.
Even if we know that the function throws only NSErrors, the compiler doesn't know that. (There are exceptions but that is a different topic .)

Now to your question how an error should be handled in general, and I would say: it depends. There are tree different scenarios:

  • Loading the strings can fail, and you want to present or log an error message in that case. Then the above method of throwing and catching an error is appropriate.
  • Loading the strings can fail, but the particular reason is of no interest to the caller. In that case I would change the function to return an optional.

Example:

func linesFromResource(fileName: String) -> [String]? {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        return nil
    }
    do {
        let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
        return content.componentsSeparatedByString("\n")
    } catch {
        return nil
    }
}

 // Usage:
if let lines = linesFromResource("file.txt") {
    print(lines)
}
  • Finally, if failing to load the strings is a programming error then the function should abort in the error case, and return the (non-optional) strings otherwise.

As an example, if this function is only used to load strings from fixed compiled-in resource files which are supposed to exist, then failing to load a file would be a programming error and should be detected early:

func linesFromResource(fileName: String) -> [String] {

    guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil) else {
        fatalError("Resource file for \(fileName) not found.")
    }
    do {
        let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
        return content.componentsSeparatedByString("\n")
    } catch let error {
        fatalError("Could not load strings from \(path): \(error).")
    }
}

let lines = linesFromResource("file.txt")
print(lines)

fatalError() prints a message before stopping execution of the program, which can be helpful to locate the programming error. Otherwise you could shorten the "forced" version to

func linesFromResourceForced(fileName: String) -> [String] {

    let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)!
    let content = try! String(contentsOfFile: path, encoding: NSUTF8StringEncoding)
    return content.componentsSeparatedByString("\n")
}

which is the "three liner" that you were looking for in the introduction to your question.

\$\endgroup\$
  • \$\begingroup\$ My main learning points from your answer: (1) avoid the non-standard "get" naming prefix, (2) use the predefined error codes when available rather than making my own, (3) let the error propagate to the caller, (4) when to use different error handling methods: (a) need message--throw/catch, (b) don't need message--optional, (c) programming error--abort immediately. You taught me a lot. Thank you very much for taking the time to do this! \$\endgroup\$ – Suragch Aug 13 '15 at 14:52
2
\$\begingroup\$
let path = NSBundle.mainBundle().pathForResource(fileName, ofType: nil)
if path == nil {
    throw ImportError.FileNotFound
}

We can clean this part up into a guard let, which not only saves us a line, but also has the advantage of making path a non-optional.

guard let path = NSBundle.mainBundle().pathForResource(fileName, ofType:nil) {
    throw ImportError.FileNotFound
}

If path would be nil, we enter the guard block and throw. Otherwise, we can treat path as a non-optional.


In fact, we can apply the guard let all the way through your method.

guard let content = try String(contentsOfFile: path, encoding: NSUTF8StringEncoding) {
    throw ImportError.CouldNotLoadFile
}

let newLineChars = NSCharacterSet.newlineCharacterSet()
let lineArray = content.componentsSeparatedByCharactersInSet(newLineChars).filter({!isEmpty($0)})

return lineArray

Note that I've got a different error and it's more accurate. Splitting a string into components either by a string or by a character set cannot return nil, nor can it throw any sort of an error.

Importantly though, I'm splitting the string on the newLineCharacterSet() rather than just the '\n' character. This will give us more accurate results regardless of what platform the file was created on.

\$\endgroup\$
  • \$\begingroup\$ Sorry for "duplicating" the guard let path = NSBundle ... part, but I had prepared that already. Your guard let content = try String ... statement seems not to compile though. \$\endgroup\$ – Martin R Aug 13 '15 at 14:01
  • \$\begingroup\$ @nhgrif, I had seen the guard keyword before but I didn't really understand how it was used until I read your answer. That was helpful. \$\endgroup\$ – Suragch Aug 13 '15 at 14:55
  • \$\begingroup\$ Unless I am mistaken, you cannot use guard to catch the error from a throw. \$\endgroup\$ – Martin R Aug 13 '15 at 18:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.