UIAlertView is dangerous
Saturday, February 25, 2012 at 11:05PM And so is UIActionSheet. Here's why. Starting with iOS 4, more and more of the standard library replaced its old delegate patterns with block-based handlers. Here's an example of animating UIViews pre iOS 4:
- (IBAction)buttonTapped:(id)sender {
[UIView beginAnimations:nil context:nil];
[UIView setAnimationDuration:1.0];
[UIView setAnimationDelegate:self];
[UIView setAnimationDidStopSelector:@selector(viewDidMove:finished:context:)];
view.center = CGPointMake(50, 50);
[UIView commitAnimations];
}
...
- (void)viewDidMode:(NSString *)animationId finished:(BOOL)finished context:(void *)context {
[self updateState];
}
And for iOS 4+ you'd just do this:
- (IBAction)buttonTapped:(id)sender {
[UIView animateWithDuration:1.0 animations:^{
view.center = CGPointMake(50, 50);
} completion:^(BOOL finished) {
[self updateState];
}];
}
Block-based handlers made it so that you'd write less code. And the code you wrote for your handler was you attached to the code that triggered the animation. Not scattered throughout your code's delegate methods.
However, UIActionSheet and UIAlertView have not been update to use blocks instead of delegates. I think it has to do with their delegate protocols having so many methods. Still, for the most part I only use - (void)dismissWithClickedButtonIndex:(NSInteger)buttonIndex animated:(BOOL)animated.
There are a few dangers of using delegates of UIAlertView and UIActionSheets. Aside from forcing you to spread out the code that's relevant to the alert view, I came across a weird bug in one of my apps. I had a UITableViewController with a list of items, which implemented UIAlertViewDelegate. I would pop a UIAlertView asking the user "Do you really want to remove the item named
- (void)alertView:(UIAlertView *)alertView didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (buttonIndex == 0) {
[self deleteItemAtIndex:self.selectedIndex];
}
}
Way later I made a subclass of the UITableViewController, and wrote some code which popped an UIAlertView on network errors:
UIAlertView *alertView = [[UIAlertView alloc] initWithTitle:@"Couldn't connect to the server." message:@"Try again in later." delegate:self cancelButtonTitle:@"OK" otherButtonTitles:nil];
[alertView show];
[alertView release];
Did you spot where I went wrong? I just filled out the initializer without really thinking about it. This was a fire-and-forget UIAlertView, but I accidentally put delegate:self in there. The subclass didn't implement UIAlertViewDelegate, but its super class did. So whenever I hit "OK", super's didDismissWithButtonIndex would be called with buttonIndex 0.
So you need to always, always, always save your UIAlertView and compare it in your delegate method:
self.theAlertView = [[[UIAlertView alloc] initWithTitle:@"Warning" message:@"Do you really want to remove the item?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.theAlertView show];
...
- (void)alertView:(UIAlertView *)alertView didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (self.theAlertView == alertView) {
if (buttonIndex == 0) {
[self deleteItemAtIndex:self.selectedIndex];
}
}
}
But what if potentially pop multiple UIAlertViews in a single stack, like with a validation method:
- (void)validateInputs {
if ([self missingTitle]) {
self.theAlertView = [[[UIAlertView alloc] initWithTitle:@"Empty title" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.theAlertView show];
}
if ([self missingBody]) {
self.theAlertView = [[[UIAlertView alloc] initWithTitle:@"Empty body" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.theAlertView show];
}
if ([self missingImage]) {
self.theAlertView = [[[UIAlertView alloc] initWithTitle:@"Missing image" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.theAlertView show];
}
}
Oops, we've overwritten the theAlertView member, so now we'll have to do something like this:
@property (readonly) NSMutableSet *alertViews;
...
- (void)validateInputs {
@synchronized (self.alertViews) {
if ([self missingTitle]) {
UIAlertView *alertView = [[[UIAlertView alloc] initWithTitle:@"Empty title" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.alertViews addObject:alertView];
[alertView show];
}
if ([self missingBody]) {
UIAlertView *alertView = [[[UIAlertView alloc] initWithTitle:@"Empty body" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.alertViews addObject:alertView];
[alertView show];
}
if ([self missingImage]) {
UIAlertView *alertView = [[[UIAlertView alloc] initWithTitle:@"Missing image" message:@"Continue?" delegate:self cancelButtonTitle:@"Yes" otherButtonTitles:@"No", nil] autorelease];
[self.alertViews addObject:alertView];
[alertView show];
}
}
}
...
- (void)alertView:(UIAlertView *)alertView didDismissWithButtonIndex:(NSInteger)buttonIndex {
@synchronzed (self.alertViews) {
if ([self.alertViews containsObject:alertView]) {
if (buttonIndex == 0) {
[self continueAddingItem];
}
[self.alertViews removeObject:alertView];
}
}
}
Do you see what a total pain in the ass this is? And the potential for bugs to creep in? And it's the same basic premise with UIActionSheets.
Here's what I'm proposing. Instead of delegates for alert views and action sheets I've implemented subclasses of both these classes, which use blocks as handlers. Now these blocks only cover one of the delegate methods: didDismissWithButtonIndex. If you need any of the other methods, then use the regular classes. For the majority of cases, you're good with didDismiss.
They're up on github, go ahead and do whatever. They're under the MIT License.
You use them like this:
[[[HSAlertView alloc] initWithTitle:@"Stuff" message:@"Do stuff?" dismissBlock:^(NSInteger buttonIndex) {
// Do stuff
} cancelButtonTitle:@"Cancel" otherButtonTitles:@"OK", @"Yes", @"No", nil] show];
[[[HSActionSheet alloc] initWithTitle:@"Do stuff?" dismissBlock:^(NSInteger buttonIndex) {
// Do stuff
} cancelButtonTitle:@"Cancel" destructiveButtonTitle:@"No" otherButtonTitles:@"Yes", @"Maybe", nil] showInView:self.view];