5
\$\begingroup\$

This works, and the UI is snappy in the simulator, but since this is my first time really using GCD, I'd just like this code to be reviewed before I start using it everywhere. Note that this is inside a PFQueryTableViewController.

My function:

 func tableRefresh() {
 // get quality of service (high level)
 let qos = Int(QOS_CLASS_USER_INITIATED.value)
 // get global queue
 dispatch_async(dispatch_get_global_queue(qos, 0)) { () -> Void in
 //execute slow task
 self.queryForTable()
 // get main queue, do UI update
 dispatch_async(dispatch_get_main_queue()) {
 self.loadObjects()
 }
 }
 }

My function in action:

@IBAction func done(segue: UIStoryboardSegue) {
 let addPersonViewController = segue.sourceViewController as! AddPersonViewController
 // If I get a person back from APVC
 if let person = addPersonViewController.person {
 // If that person has a name
 if let name = person.name {
 // Note: the reference to current user creates the pointer
 let newPerson = Person(name: name, user: PFUser.currentUser()!)
 // Save
 newPerson.saveInBackgroundWithBlock() { succeeded, error in
 if succeeded {
 println("\(newPerson) was Saved")
 self.tableRefresh()
 } else {
 if let errorMessage = error?.userInfo?["error"] as? String {
 self.showErrorView(error!)
 }
 }
 }
 }
 }
 }
Mast
13.8k12 gold badges55 silver badges127 bronze badges
asked Jul 30, 2015 at 16:11
\$\endgroup\$
1
  • \$\begingroup\$ I recommend reposting this question after taking into account all points made in @Hosch250's answer. \$\endgroup\$ Commented Aug 4, 2015 at 1:07

1 Answer 1

4
\$\begingroup\$

First off, I don't know Swift. These are generic issues that should be addressed in all written code, and I'll leave the Swift-specific stuff to the experts.

Comments

//execute slow task

Comments should say why the code is the way it is, not a generic comment that doesn't tell us anything constructive. If there is an issue with the code being slow, document why it is slow, and maybe state why you haven't been able to fix it for future reference.

Spacing

 }
}

We definitely don't need that much whitespace hogging our screens. It forces code off the screen, it increases scrolling, and generally slows us down when reading and understanding the code.

Indentation

} else {
 if let errorMessage = error?.userInfo?["error"] as? String {
 self.showErrorView(error!)
 }
 }

Your indentation is off there. Each closing brace should have the same level indentation as the opening brace, unless they are on the same line, which is rare.

Naming

Your names should state what the function does or variable is, and only what the function does or variable is, and the variable/function shouldn't be/do anything but what the name says it is/does.

done() tells me nothing about what the function does.

qos doesn't tell me what the variable does, what it contains, or what it is used for.

self.loadObjects() appears to do UI updates, according to the comment. Loading an object is not the same as displaying it, and loading and displaying from a single method violates the Single Responsibility Principle.

answered Jul 31, 2015 at 0:40
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.