Checking for many alertable conditions before saving customer information

Question

What's the best way to refactor this code? The method does a lot of checking before showing an alert. And the method is too long for my taste.

I'm thinking of extracting the checkers into their own methods, but I'm not sure how to handle multiple instances that call return.

- (IBAction)saveCustomer:(id)sender {
    @try {
        [self.view endEditing:YES];

        // Cannot save if customer is pending
        if ([self.service isWaitingForApproval]) {
            APAlert(WARNING_TITLE, NSLocalizedString(@"message.waiting-approval", nil));
            return;
        }

        // Check required field
        NSString *incompleteRequiredField = [self.service checkRequiredFields];
        if (incompleteRequiredField) {
            NSString *message = [NSString localizedStringWithFormat:NSLocalizedString(@"message.required field %@", nil), NSLocalizedString(incompleteRequiredField, nil)];
            APAlert(WARNING_TITLE, message);
            return;
        }

        if(![self.service isDistributorOrWholesalerInTW]) {
            incompleteRequiredField = [self checkRequireCellOTM];
            if(incompleteRequiredField) {
                NSString *message = [NSString localizedStringWithFormat:NSLocalizedString(@"message.required otm %@", nil), NSLocalizedString(incompleteRequiredField, nil)];
                APAlert(WARNING_TITLE, message);
                return;
            }
        }

        Country_Identifier countryID = (Country_Identifier)[self.service.customer.country_id integerValue];
        if (countryID == Country_Taiwan || countryID == Country_Thailand) {
            if (![self.service isHasContact]) {
                APAlert(WARNING_TITLE, ALERT_CONFRIM_HAVE_CONTACT);
                return;
            }
        }
        [[SomeManager alertViewManager] showAlertWithTitle:SAVING_TITLE
                                                                  message:SAVE_CUSTOMER_CONFIRM
                                                                   target:self
                                                               actionView:nil
                                                        cancelButtonTitle:YES_BUTTON_TITLE
                                                        otherButtonTitles:NO_BUTTON_TITLE
                                                                   andTag:AlertViewTag_Save];
    }
    @catch(NSException *exception) {
        [self sendException:exception];
    }
}

Answer 1

You can absolutely extract validation part out of this method because almost every validation is depending on self.service object.

1 Write a method in self.service class that will validate situation and return NSString message if not valid situation.

2 As only one validation is happening on self i.e. checkRequireCellOTM method, pass result of this method while validating self.service.

3 According to result of new method show an alert or proceed.

Here is how I'm trying to clean it-

New method in class of self.service:

- (NSString*)validateServiceReadiness:(NSString*)requiredCellOTM {
  // Cannot save if customer is pending
  if ([self isWaitingForApproval]) {
    return NSLocalizedString(@"message.waiting-approval", nil);
  }

  // Check required field
  NSString *incompleteRequiredField = [self checkRequiredFields];
  if (incompleteRequiredField) {
    NSString *message = [NSString localizedStringWithFormat:NSLocalizedString(@"message.required field %@", nil), NSLocalizedString(incompleteRequiredField, nil)];
    return message;
  }

  if(![self isDistributorOrWholesalerInTW]) {
    incompleteRequiredField = requiredCellOTM;
    if(incompleteRequiredField) {
        NSString *message = [NSString localizedStringWithFormat:NSLocalizedString(@"message.required otm %@", nil), NSLocalizedString(incompleteRequiredField, nil)];
        return message;
    }
  }

  Country_Identifier countryID = (Country_Identifier)[self.customer.country_id integerValue];
  if (countryID == Country_Taiwan || countryID == Country_Thailand) {
    if (![self isHasContact]) {
        return ALERT_CONFRIM_HAVE_CONTACT;
    }
  }
}

Then use this method for validation as:

- (IBAction)saveCustomer:(id)sender {
  @try {
    [self.view endEditing:YES];

    NSString *serviceReadinessMessage = [self.service validateServiceReadiness:[self checkRequireCellOTM]];
    if (serviceReadynessMessage != nil) {
        APAlert(WARNING_TITLE, serviceReadinessMessage);
        return;
    }
    [[SomeManager alertViewManager] showAlertWithTitle:SAVING_TITLE
                                               message:SAVE_CUSTOMER_CONFIRM
                                                target:self
                                            actionView:nil
                                     cancelButtonTitle:YES_BUTTON_TITLE
                                     otherButtonTitles:NO_BUTTON_TITLE
                                                andTag:AlertViewTag_Save];
  }
  @catch(NSException *exception) {
    [self sendException:exception];
  }
}

I'm not sure which method is throwing an exception here, but if it's belongs to our new method validateServiceReadiness then modify syntax accordingly.

Answer 2

Move all this service logic out of your VC and just use NSError to pass failure messages back to the user.

- (IBAction)saveCustomer:(id)sender
{
    NSError *error;
    if (![self.service canSave:&error]) {
        // Present alert using error.localizedDescription as the message
    } else {
        // Present save confirmation
    }
}

Or even better just wait until the user actually tries to save

UIAlertAction *saveAction = [UIAlertAction actionWithTitle:NSLocalizedString(@"Save", nil) style:UIAlertActionStyleDefault handler:^(UIAlertAction * _Nonnull action) {
    NSError *error;
    if (![self.service save:&error]) {
        UIAlertController *alertController = [UIAlertController alertControllerWithTitle:NSLocalizedString(@"Could Not Save", nil) message:error.localizedDescription preferredStyle:UIAlertControllerStyleAlert];
        [alertController addAction:[UIAlertAction actionWithTitle:NSLocalizedString(@"Ok", nil) style:UIAlertActionStyleDefault handler:nil]];
        [self presentViewController:alertController animated:YES completion:nil];
    }
}];