I have the following code that displays the alerts depending on the result: How can I improve the following code:
-(IBAction)saveSettings:(id)sender
{ UIAlertView *alert = nil;
username = self.usernameTextField.text;
token = self.passwordTextField.text;
// validate the username and token
if(![self isValid])
{
// show alert that it is not valid
alert = [[UIAlertView alloc] initWithTitle:@"Error" message:@"Invalid User Name or Password" delegate:self can开发者_高级运维celButtonTitle:nil otherButtonTitles:@"Ok", nil];
[alert show];
return;
}
BOOL isSynched = [self syncSettings];
if(!isSynched)
{
alert = [[UIAlertView alloc] initWithTitle:@"Error" message:@"Error Syncing Settings" delegate:self cancelButtonTitle:nil otherButtonTitles:@"Ok", nil];
[alert show];
}
else
{
alert = [[UIAlertView alloc] initWithTitle:@"" message:@"Settings has been Synced Syncing" delegate:self cancelButtonTitle:nil otherButtonTitles:@"Ok", nil];
[alert show];
}
}
I think I am instantiating the alert too many times and looks kind of repetitive!
username = self.usernameTextField.text;
token = self.passwordTextField.text;
UIAlertView* alert = [[UIAlertView alloc] initWithTitle:@"Error"
message:@""
delegate:self
cancelButtonTitle:nil
otherButtonTitles:@"Ok", nil];
[alert autorelease];
// validate the username and token
if(![self isValid])
{
// show alert that it is not valid
alert.message = @"Invalid User Name or Password";
[alert show];
return;
}
BOOL isSynched = [self syncSettings];
if(!isSynched)
{
alert.message = @"Error Syncing Settings";
}
else
{
alert.title = @"";
alert.message = @"Settings has been Synced Syncing";
}
[alert show];
Try leaving all those instances of [alert show];
out & then at the end put...
if (!alert) return;
[alert show];
phix23's answer also provides good improvement because you're setting a bunch of the properties of the alert explicitly & only once. If you use that you can also use mine by changing the condition to...
if (alert.message != @"") return;
Cheers, Pedro :)
精彩评论