I have just started really getting into iOS and wondered if someone could tell me if this is short piece of code is decent or not? I am trying to parse some JSON which has the following format:
I am just trying to take the score from the json and reformat it and eventually put it into a label.
override func viewDidLoad() {
super.viewDidLoad()
// Do any additional setup after loading the view, typically from a nib.
let url = NSURL(string: "http://api.dev/web/game-view")!
let task = NSURLSession.sharedSession().dataTaskWithURL(url) { (data, response, error) in
if let urlContent = data {
do {
let json: NSArray = try NSJSONSerialization.JSONObjectWithData(urlContent, options: NSJSONReadingOptions.MutableContainers) as! NSArray
if json.count > 0 {
for j in json {
let home_score = j["home_score"] as? Int
let away_score = j["away_score"] as? Int
if let h = home_score {
if let a = away_score {
print("\(h) - \(a) ")
}
}
}
}
} catch {
print("Error")
}
}
// need error clauses
}
task.resume()
}
It’s decent enough. Assuming that this does all you want it to, I’d take advantage of some of Swift’s features to streamline it a bit.
First: your JSON format is an array of objects. Let’s make your code type-safe: each object is a [String: AnyObject] dictionary, and the array is a [[String: AnyObject]] - prefer this over an NSArray.
Second, let’s make it easier to tell which parts of your code will be run. ‘task.resume()’ will be run after everything, regardless, so let’s put it up top in a defer closure. Then we’ll use guard let and do-try-catch to ensure that if data is nil, if the JSON parsing doesn’t work, or if the output is not an array of dictionaries, then you want to return early.
Third, simple one: if json.count > 0 is redundant. for j in json will work through every element in the array; if there are no elements, then your code will iterate through that loop no times.
Fourth: you can combine the nested if let statements you have into one if let. You can also remove the word NSJSONReadingOptions as the compiler knows the enum type it’s expecting.
Fifth: I like descriptive variable names. j, h, a is fine if you’re prototyping quickly, if they’re genuinely throwaway variables, or if there are standard associations (e.g. ‘x’ and ‘y’ as co-ordinate labels) but you might make this code longer and then have to re-read it in a month’s time and you might wonder what all the letters stand for.
So here’s my refactored version of your code:
override func viewDidLoad() {
super.viewDidLoad()
// Do any additional setup after loading the view, typically from a nib.
let url = NSURL(string: "http://api.dev/web/game-view")!
let task = NSURLSession.sharedSession().dataTaskWithURL(url) { (data, response, error) in
defer { task.resume() }
guard let urlContent = data
else { return }
do {
let jsonObject = try NSJSONSerialization.JSONObjectWithData(urlContent, options: .MutableContainers)
guard let jsonArray = jsonObject as? [[String: AnyObject]]
else { return }
for dictionary in jsonArray {
if let home_score = dictionary["home_score"] as? Int,
away_score = dictionary["away_score"] as? Int {
print("\(home_score) - \(away_score)")
}
}
} catch {
print("Error")
}
}
}
Thank you for this, I’m trying to get used to Swift and dealing with optionals properly so thanks for this. I altered my code, but my defer statement defer { task.resume() } i got “variable used within it’s own initial value”? I assumed wrongly that referencing it as self.task.resume() would stop that error, but it didn’t.
I apologise. I refactored without testing this to see if it all worked, and I missed where the resume call was - it’s outside the closure, after you define ‘task’ with the closure - and I got a bit too clever for my own good with the Swift 2 features.
Forget the defer line of code. Leave the ‘task.resume()’ call where it was. (Prefixing it with ‘self.’ was a good thought but ‘task’ is a local variable, not a property of your view controller, so it won’t work.)
So… this instead (I hope this works.)
override func viewDidLoad() {
super.viewDidLoad()
// Do any additional setup after loading the view, typically from a nib.
let url = NSURL(string: "http://api.dev/web/game-view")!
let task = NSURLSession.sharedSession().dataTaskWithURL(url) { (data, response, error) in
guard let urlContent = data
else { return }
do {
let jsonObject = try NSJSONSerialization.JSONObjectWithData(urlContent, options: .MutableContainers)
guard let jsonArray = jsonObject as? [[String: AnyObject]]
else { return }
for dictionary in jsonArray {
if let home_score = dictionary["home_score"] as? Int,
away_score = dictionary["away_score"] as? Int {
print("\(home_score) - \(away_score)")
}
}
} catch {
print("Error")
}
} // end of 'let task ='
task.resume()
}
Sorry about that. I hope the rest made sense though.