While using a UIActionSheet in an iphone app, the typical methods of matching actions to buttons seem very fragile and aesthetically unpleasant. Perhaps its due to my minimal C/C++ background (more Perl, Java, Lisp and others). Matching on button indexes just seems like too many magic numbers and too disconnected to avoid simple logical or consistency errors.
For instance,
UIActionSheet *sources = [[UIActionSheet alloc]
initWithTitle:@"Social Networks"
delegate:self
cancelButtonTitle:@"Cancel"
destructiveButtonTitle:nil
otherButtonTitles:@"Twitter", @"Facebook", @"Myspace", @"LinkedIn", @"BlahBlah", nil
];
<snip>
-(void)actionSheet:(UIActionSheet *)actionSheet didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (buttonIndex == [actionSheet cancelButtonIndex]) {
// all done
} else if (buttonIndex == 0) {
// Twitter
} else if (buttonIndex == 1) {
// Facebook
} else if (buttonIndex == 2) {
// LinkedIn
} else if (buttonIndex == 3) {
// Myspace
}
}
Notice there's at least two errors in the action handling code (according to the comments at least).
What I'm missing is the correct design pattern for avoiding that disconnect in Objective-C. If this were perl, I would first build an array of my button options and then probably create a quick lookup table hash that would correspond to another lookup table of objects or subroutines that did the appropriate thing for each item. In java, the original list would probably be objects in the first place with callbacks. I know I could build a Dictionary to mimic the perl hash but that feels very unwieldy and cumbersome for 3-4 options. I've also considered using a enum to mask the magic-ness of indexes but that's only a minor part of the problem.
The real problem seems to be that there's no (simple?) way to specify BOTH the list of button strings and corresponding actions in one place thereby eliminating the need to modify code in two places when adding/removing/reordering options and thus making it effectively impossible to make the kinds of mistakes that my sample code makes.
I'm not trying to start a programming language holy war, I just want to figure what the correct design pattern in this scenario (and I believe many others in Objective C) for connecting the list of button strings to the list of actions.
I prefer this way
- (void)actionSheet:(UIActionSheet *)actionSheet didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (buttonIndex == [actionSheet cancelButtonIndex])
{
// cancelled, nothing happen
return;
}
// obtain a human-readable option string
NSString *option = [actionSheet buttonTitleAtIndex:buttonIndex];
if ([option isEqualToString:@"Twitter"])
{
//...
} else if ([option isEqualToString:@"FaceBook"])
{
//...
}
}
I completely agree with the question. Apple's design here encourages the use of magic numbers, and I was a little shocked to see all the solutions out there recommending use of hardcoded numbers for the button indexes.
Here's my solution for Swift.
- Create an enum containing an item for every button title, like:
enum ImagePickerActionSheetButtons
{
case Camera
case Chooser
}
Populate a dictionary with the LOCALIZED strings for each button title, where the keys are items from the enum:
// Populate with LOCALIZED STRINGS
var buttonTitles:[ImagePickerActionSheetButtons:String] =
[ImagePickerActionSheetButtons.Camera:"Take photo",
ImagePickerActionSheetButtons.Chooser :"Choose photo"]
Create the action sheet, getting the button titles from the dictionary by their enum value:
func createActionSheet()->UIActionSheet
{
var sheet: UIActionSheet = UIActionSheet()
sheet.addButtonWithTitle(buttonTitles[.Camera]!)
sheet.addButtonWithTitle(buttonTitles[.Chooser]!)
sheet.addButtonWithTitle("Cancel")
sheet.cancelButtonIndex = sheet.numberOfButtons - 1
sheet.delegate = self
return sheet
}
Finally, in the clickedButtonAtIndex code, check the title of the clicked button against the localized strings in the dictionary:
func actionSheet(sheet: UIActionSheet!, clickedButtonAtIndex buttonIndex: Int)
{
if (sheet.buttonTitleAtIndex(buttonIndex) == buttonTitles[.Camera]!)
{
takePhoto()
}
else if (sheet.buttonTitleAtIndex(buttonIndex) == buttonTitles[.Chooser]!)
{
choosePicFromLibrary()
}
else if (buttonIndex == sheet.cancelButtonIndex)
{
// do nothing
}
}
Maybe you could put the actions for the buttons in an array
actionsArray = [NSMutableArray arrayWithObjects: @selector(btn1Clicked),
@selector(btn2Clicked),
@selector(btn3Clicked),
@selector(btn4Clicked), nil];
then in didDismissWthButtonIndex
-(void)actionSheet:(UIActionSheet *)actionSheet didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (buttonIndex == [actionSheet cancelButtonIndex]) {
// all done
} else {
[this [actionsArray objectAtIndex: buttonIndex]];
}
}
I am pretty certain you could put a more complex object in the array including button info and method and then have it all contained in the array. Probably better error checking on the index of the array....etc
To be honest with you I never thought about this pattern much until i read the question so this is just off the top of my head
How about that one?
In that way, you don't have to worry about indexes since buttons and actions are added in the same place.
typedef void (^contact_callback_t)(MyContactsController *controller);
…
- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
NSDictionary *contact = [myContacts objectAtIndex:indexPath.row];
UIActionSheet *_actionSheet = [[UIActionSheet alloc] initWithTitle:NSLocalizedString(@"Contact Action", @"")
delegate:self
cancelButtonTitle:nil
destructiveButtonTitle:nil
otherButtonTitles:nil];
_actions = [NSMutableArray new];
if([contact objectForKey:@"private_email"] != nil) {
[_actionSheet addButtonWithTitle:
[NSString stringWithFormat:NSLocalizedString(@"E-Mail: %@", @""), [contact objectForKey:@"private_email"] ] ];
contact_callback_t callback = ^(MyContactsController *controller) {
[controller openEmail:contact];
};
[_actions addObject:callback];
}
if([contact objectForKey:@"private_telefon"] != nil) {
[_actionSheet addButtonWithTitle:
[NSString stringWithFormat:NSLocalizedString(@"Phone: %@", @""), [contact objectForKey:@"private_telefon"] ]];
contact_callback_t callback = ^(MyContactsController *controller) {
[controller dial:[contact objectForKey:@"private_telefon"]];
};
[_actions addObject:callback];
}
[_actionSheet showFromTabBar:tabBar];
}
- (void)actionSheet:(UIActionSheet *)actionSheet clickedButtonAtIndex:(NSInteger)buttonIndex
{
if(buttonIndex == actionSheet.cancelButtonIndex)
{
}
else
{
contact_callback_t callback = [_actions objectAtIndex:buttonIndex];
callback(self);
}
_actions = nil;
}
Building on Aaron's selector suggestion, I really like the idea now of doing a simple ad-hoc dispatch method. It successfully avoids the possibility of handling the wrong option as well as providing a clean factorization of concerns. Of course, I could imagine a use case where you want to do something else first for each option, such as instantiating an object and passing it the option string much like Toro's answer.
Here's a simple dispatch that calls methods such as 'actionTwitter':
-(void)actionSheet:(UIActionSheet *)actionSheet didDismissWithButtonIndex:(NSInteger)buttonIndex {
if (buttonIndex == [actionSheet cancelButtonIndex]) {
return;
}
NSString *methodName = [@"action" stringByAppendingString:[actionSheet buttonTitleAtIndex:buttonIndex]];
SEL actionMethod = NSSelectorFromString(methodName);
if ([self respondsToSelector:actionMethod]) {
[self performSelector:actionMethod];
} else {
NSLog(@"Not yet implemented")
}
}
