patternswiftMinor
Checking two text field for match and checking for null
Viewed 0 times
fieldnulltextcheckingmatchtwoforand
Problem
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.")
}
}Solution
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
I don't understand why we're
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
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.
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:
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
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.Code Snippets
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
}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)
}Context
StackExchange Code Review Q#90062, answer score: 9
Revisions (0)
No revisions yet.