Swift API controller

Question

I've just started iOS development and learning Swift. My code below works, but I'm interested to know in ways I can improve it having no experience in iOS Swift prior to this attempt.

import UIKit
class NewsViewController: UIViewController, UITableViewDataSource, UITableViewDelegate {

var tableView: UITableView!
var titleItems: NSMutableArray = []
var descritpionItems: NSMutableArray = []

/*
class to get data from API
*/
class RemoteAPI {

    // base url
    let baseUrl = "http://api.dev/web/"
    var newsTitle: NSMutableArray = []
    var newsDescription: NSMutableArray = []

    func getData(completionHandler: ((NSArray!, NSError!) -> Void)!) -> Void {
        // get news feed url
        let url = NSURL(string: baseUrl + "news")
        // create session
        let session = NSURLSession.sharedSession()
        // set task
        let task = session.dataTaskWithURL(url!, completionHandler: {data, response, error -> Void in
            if (error != nil) {
                return completionHandler(nil, error)
            }

            var error: NSError?
            // convert data object to json
            let json = JSON(data: data)

            for var i = 0; i < json.count; ++i {
                // get news title from json object
                var title = json[i]["title"].string
                var blurb = json[i]["description"].string
                // add to dictionary
                self.newsTitle.addObject(title!)
                self.newsDescription.addObject(blurb!)
            }

            if (error != nil) {
                return completionHandler(nil, error)
            } else {
                return completionHandler([self.newsTitle,self.newsDescription], nil)
            }
        })
        task.resume()


    }
}

// get data
var api = RemoteAPI()

override func viewDidLoad() {
    super.viewDidLoad()
    // Do any additional setup after loading the view.
    self.view.frame = CGRect(x: 0, y: 50, width: 320, height: 480)
    self.tableView = UITableView(frame:self.view!.frame)
    self.tableView!.delegate = self
    self.tableView!.dataSource = self
    self.tableView!.registerClass(UITableViewCell.self, forCellReuseIdentifier: "cell")
    self.view?.addSubview(self.tableView)

    api.getData({data, error -> Void in
        if (data != nil) {
            self.titleItems = self.api.newsTitle
            self.descritpionItems = self.api.newsDescription
            self.tableView!.reloadData()
        } else {
            println("API failed :-(")
            println(error)
        }
    })
}
func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int{

    return self.titleItems.count
}

func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell{

    var cell:UITableViewCell=UITableViewCell(style: UITableViewCellStyle.Subtitle, reuseIdentifier: "mycell")

    if let newsTitle = self.titleItems[indexPath.row] as? NSString {
        cell.textLabel?.text = newsTitle
    }
    if let newsDesc = self.descritpionItems[indexPath.row] as? NSString {
        cell.detailTextLabel?.text = newsDesc
    }
    return cell
}
}

Answer 1

There's quite a lot to address here, but I'm going to start with my absolute largest concern--one that will cause problems for anyone who uses this:

func getData(completionHandler: ((NSArray!, NSError!) -> Void)!) -> Void

There are places where it's okay to use the implicitly unwrapped optional !, but this is not one of those places by any means.

First of all, you use it on the completion handler as a whole, which means it's somehow okay for the user to call:

getData(nil)

But you never check whether or not a completion handler was passed to you. You just assume one was and call it anyway, which means any time someone calls this method and passes nil, there will be a crash.

But the arguments the completion handler takes also should not be implicitly unwrapped options. They should just be regular optionals: ?. This is especially true here as we are explicitly passing nil into the completion handler in some cases. Implicitly unwrapped optionals are for very special cases. You need to use the regular optional which tells any user "YOU MUST CHECK THIS FOR NIL!". Implicitly unwrapped optionals say "This object is never nil so never bother checking it! It's only an optional because of some extraordinarily special scenario that's handle entirely by the internal class in initialization."

If I start using your RemoteAPI class, I'd never check the objects in the completion handler for nil (until I got a crash).

Unless you're Apple making Objective-C code in their massive libraries work correctly with Swift (and even here, Apple is only using ! as a placeholder and intends to replace these all with either non-optionals or regular optionals) OR you're writing something where a property can be nil in the init sometimes, then you don't need to be using the implicitly unwrapped optional! It looks like you've got a handleful of implicitly unwrapped optionals that need to be removed.