patternswiftMinor
Swift API controller
Viewed 0 times
swiftapicontroller
Problem
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 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? NS
```
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 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? NS
Solution
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:
There are places where it's okay to use the implicitly unwrapped optional
First of all, you use it on the completion handler as a whole, which means it's somehow okay for the user to call:
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
But the arguments the completion handler takes also should not be implicitly unwrapped options. They should just be regular optionals:
If I start using your
Unless you're Apple making Objective-C code in their massive libraries work correctly with Swift (and even here, Apple is only using
func getData(completionHandler: ((NSArray!, NSError!) -> Void)!) -> VoidThere 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.Code Snippets
func getData(completionHandler: ((NSArray!, NSError!) -> Void)!) -> VoidgetData(nil)Context
StackExchange Code Review Q#84746, answer score: 4
Revisions (0)
No revisions yet.