Proper way to connect UIActionSheet options to actions

Question

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.

Whoremaster

Answer 1

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")
    }
}

Whoremaster

You are genius. I doesn't think about this way. – Roseannroseanna

This pretty much guarantees you won't be able to translate your app's UI into another language. – Rockabilly

This is the worst way in the entire topic. You won't be able to localize your buttons, and it produces a warning on performSelector. performSelector should always be used with a static selector (non dynamic). However, your question is a good question. – Sporophore

Answer 2

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"])
    {
        //...
    }
}

Roseannroseanna

That's a definite improvement over matching on the indexes. – Whoremaster

Don't forget localization - compare using the same strings you're putting in the UI, which presumably come from NSLocalizedString. – Rockabilly

Answer 3

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
    }
}

Thackeray

Answer 4

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

Rehash

I was pretty sure you couldn't use an array in place of a nil-terminated argument list. But that said, I had no idea you could reference methods like that. That's getting very close to what I was looking for. – Whoremaster

Answer 5

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;
}

Gillen