Checking two text field for match and checking for null

Question

Is this the most efficient way to do this? Any suggestions on some thing I might have missed or can do better?

    func checkPasscodeMatch(){
    println(self.passCodetext1.text)
    println(self.passCodetext2.text)
    // check if pass codes match
    if self.passCodetext1.text == self.passCodetext2.text {
    //check if pass code 1 is blank
        if self.passCodetext1.text == "" {
            let alertView = UIAlertController(title: "ALERT!!", message: "Pass code can not be blank", preferredStyle: UIAlertControllerStyle.Alert)
            alertView.addAction(UIAlertAction(title: "OK", style: UIAlertActionStyle.Default, handler: nil))

            self.presentViewController(alertView, animated: true, completion: nil)
        } else {
       defaults.setObject(passCodetext1.text, forKey: "passCode")//store pass code
        dispatch_async(dispatch_get_main_queue()) {
            self.performSegueWithIdentifier("setupTouchID", sender: self)
            }
        }
    } else {
        let alertView = UIAlertController(title: "ALERT!!", message: "Pass Codes do not match", preferredStyle: UIAlertControllerStyle.Alert)
        alertView.addAction(UIAlertAction(title: "OK", style: UIAlertActionStyle.Default, handler: nil))

        self.presentViewController(alertView, animated: true, completion: nil)
        passCodetext1.text = ""
        passCodetext2.text = ""
        passCodetext2.resignFirstResponder()
        println("Pass codes do not match.")
    }
}

Answer 1

Your indentation is a bit wonky. Why is the first line indented? Why isn't the stuff in the inner-else indented correctly?

---

There's no reason to println() our passcodes. At the very least, we should be certain to wrap these in #if DEBUG so that they're taken out of the release build of our project.

---

I don't understand why we're dispatch_async-ing to the main thread. Is this code not already being executed on the main thread? If this isn't on the main thread, then okay... but now I have to ask how come the presentViewController calls aren't also being dispatched to the main thread? Seems like dispatching to the main thread is unnecessary.

---

As for efficiency, I'm not sure there's a way to make your code execute any faster, however, there are some improvements we can make.

First of all, rather than checking if our string is equal to "", we should check its length. If a text field has never had its text property set or become first responder, it's text property can actually be nil. "" and nil are different.

But perhaps more importantly, our conditions are structure in such a way that it makes it difficult for the programmer to sort through everything that's going on.

Let's start by handling all of our failure conditions.

var errorDescription: String?

if !(count(self.passcodeText1.text) > 0) {
    errorDescription = "Pass code can not be blank."
} else if self.passcodeText1.text != self.passcodeText2.text {
    errorDescription = "Pass codes do not match."
}

if let message = errorDescription {
    // handle error condition
} else {
    // proceed
}

Now we've simplified things a bit, eh?

Finally, let's wait until the user dismisses the alert controller away before we change anything on the current controller. This will make it more clear to the user what area is wrong. And instead of just resigning first responder, let's make the first text field the first responder so it's ready for the user to start fixing the error. Ready?

So, filling in the comments from the above snippet:

if let message = errorDescription {
    let action = UIAlertAction(title:"OK", stlye:.Default) {
        _ in
        passCodetext1.text = ""
        passCodetext2.text = ""
        passCodetext1.becomeFirstResponder()
    }

    let alertView = UIAlertController(title:"ALERT!!", message:message, preferredStyle:.Alert)
    alertView.addAction(UIAlertAction(title:"OK", style:.Default, handler: action))

    self.presentViewController(alertView, animated:true, completion:nil)
} else {
    self.performSegueWithIdentifier("setupTouchID", sender: self)
}

---

And as a final note, I'd recommend declaring all of your strings as constants in some file somewhere in your app where you store all of these. Or perhaps better yet, make a .plist file in your project to store all these in.