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!)
                        }
                    }
                }
            }

        }




    }
\$\endgroup\$
1
  • \$\begingroup\$ I recommend reposting this question after taking into account all points made in @Hosch250's answer. \$\endgroup\$
    – nhgrif
    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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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