iOS App ViewController

Question

How can I most effectively refactor the code in these 2 ViewControllers?

I'm familiar with subclassing in Objective-C and have used it extensively else where with NSObjects and UIViews, but I'm not sure how to approach doing so with ViewControllers.

The rooms have fairly similar functions, but I think are different enough to warrant 2 separate VCs.

Note: I plan to make more descriptive variable names and change a lot of the NSString messages with constants, those are just place holders for now. Also, you can ignore the log statements.

Edit:

Overview & Screenshot

• Overview: This is a video conferencing app. Both VC's are pushed by a tableview.
• In DetailVC: User is matched up against an opponent and they talk for 60 seconds in a "SRRoom".
• In Observe: A user joins a SRRoom where 2 people are already matched up and can watch them talk.

TacoVideoHandler abstracts all the nitty gritty video methods.

SRObserveViewController.h:

#import <UIKit/UIKit.h>
#import "SRTacoVideoHandler.h"
#import "SRAPI.h"
#import "SRRoom.h"
#import "SRAnimationHelper.h"

@interface SRObserveViewController : UIViewController
@property (weak, nonatomic) IBOutlet UILabel *roomTitle;
@property (weak, nonatomic) IBOutlet UIView *agreeShoutContainer;
@property (weak, nonatomic) IBOutlet UIView *disagreeShoutContainer;
@property (weak, nonatomic) IBOutlet UILabel *statusLabel;

@property (weak, nonatomic) NSTimer *retryTimer;

@property (strong, nonatomic) SRTacoVideoHandler *TacoHandler;
@property (strong, nonatomic) SRRoom *room;


@end 

SRObserveViewController.m:

#import "SRObserveViewController.h"

@interface SRObserveViewController ()

@property (strong, nonatomic) NSString *kApiKey;
@property (strong, nonatomic) NSString *kSessionId;
@property (strong, nonatomic) NSString *kToken;

@end



@implementation SRObserveViewController

- (void)viewDidLoad {
    [super viewDidLoad];
    [self configOpentTok];
    [self performGetRoomRequest];
    [self configNavBar];
    [self configNotifcations];
}

- (void)configOpentTok {
    self.TacoHandler.shouldPublish = NO;
    self.TacoHandler.isObserving = YES;
    [self.TacoHandler registerUserVideoStreamContainer:self.agreeShoutContainer];
    [self.TacoHandler registerOpponentOneVideoStreamContainer:self.disagreeShoutContainer];
}

- (NSString *)opposingPosition:(NSString *)position {
    return ([position isEqualToString:@"agree"]) ? @"disagree" : @"agree";
}

- (void)configNavBar {
    self.navigationItem.leftBarButtonItem = [[UIBarButtonItem alloc] initWithBarButtonSystemItem:UIBarButtonSystemItemDone target:self action:@selector(pressBackButton)];
    UIImage *backButtonImage = [UIImage imageNamed:@"backButton"];
    UIButton *backButton = [UIButton buttonWithType:UIButtonTypeCustom];
    [backButton setFrame:CGRectMake(0, 0, 47, 32)];
    [backButton setImage:backButtonImage forState:UIControlStateNormal];
    [backButton addTarget:self action:@selector(pressBackButton) forControlEvents:UIControlEventAllEvents];
    UIBarButtonItem *navBackButton = [[UIBarButtonItem alloc] initWithCustomView:backButton];

    [self.navigationItem setLeftBarButtonItem:navBackButton];
}

- (void)pressBackButton {
    self.navigationItem.leftBarButtonItem.enabled = NO;
    [self doCloseRoom];

    [self.TacoHandler safetlyCloseSession];
    double delayInSeconds = 3;
    //[self updateStatusLabel:@"Disconnecting" withColor:[UIColor grayColor]];

    dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delayInSeconds * NSEC_PER_SEC));
    dispatch_after(popTime, dispatch_get_main_queue(), ^(void) {
        [self.navigationController popViewControllerAnimated:YES];
    });
}

- (void)configNotifcations {
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(recieveNotifications:)
                                                 name:kSRTacoVideoHandlerNotifcations
                                               object:nil
     ];
}

- (void)recieveNotifications:(NSNotification *)notification {
    if ([[notification name] isEqualToString:kSRTacoVideoHandlerNotifcations]) {
        NSDictionary *userInfo = notification.userInfo;
        NSNumber *message = [userInfo objectForKey:@"message"];
        [self statusMessage:message];
    }
}

- (void)statusMessage:(NSNumber *)message {
    NSString *result = nil;

    switch ([message intValue]) {
        case 0:
            [self startRetryTimer];
            result = @"Disconnected";
            break;

        case 1:
            [self stopRetryTimer];
            result = @"Connecting...";
            break;

        case 3:
            result = @"Searching for Idiots...";
            [self startRetryTimer];
            break;

        case 4:
            [self stopRetryTimer];
            result = @"Wow, look at that guy...";
            break;

        case 88:
            [self stopRetryTimer];
            result = @"Observing specimens";
            break;

        case 5:
            [self stopRetryTimer];
            [self performSelector:@selector(retry) withObject:nil afterDelay:6];
            result = @"Match Over! Will start searching for new room shortly...";
            break;

        case 6:
            //[self stopProgressBar];
            result = @"Disconnecting...";
            [self stopRetryTimer];
            break;

        case 7:
            [self startRetryTimer];
            result = @"Opponent failed to Join. Will start searching for new room shortly...";
            break;

        case 77:
            //Call succeedded but no rooms are available
            result = @"Searching for Idiots...";
            [self startRetryTimer];
            break;

        case 99:
            [self stopRetryTimer];
            result = @"Everyone Left! Will start searching for new room shortly...";
            [self performSelector:@selector(retry) withObject:nil afterDelay:6];
            break;

        default:
            result = @"Retry";
    }


    [self updateStatusLabel:result withColor:[self statusLabelColorPicker:message] animated:YES];
    NSLog(@"STATUS LABEL UPDATE: %@", message);
}

- (void)startRetryTimer {
    NSLog(@"Timer Started");
    [self stopRetryTimer];
    self.retryTimer  = [NSTimer scheduledTimerWithTimeInterval:20
                                                        target:self
                                                      selector:@selector(retry)
                                                      userInfo:nil
                                                       repeats:YES];
}

- (void)stopRetryTimer {
    [self.retryTimer invalidate];
    self.retryTimer = nil;
}

- (void)retry {
    [self doCloseRoom];
    [self.TacoHandler safetlyCloseSession];
    [self performSelector:@selector(performGetRoomRequest) withObject:nil afterDelay:4];
}

- (UIColor *)statusLabelColorPicker:(NSString *)Message {
//will change this later
    return [UIColor blackColor];
}

- (void)performGetRoomRequest {
    //parameter with session
    __weak typeof(self) weakSelf = self;

    [[RKObjectManager sharedManager] getObject:weakSelf.room
                                          path:nil
                                    parameters:nil
                                       success: ^(RKObjectRequestOperation *operation, RKMappingResult *mappingResult) {
                                           weakSelf.roomTitle.text = weakSelf.room.title;
                                           if (weakSelf.room.token.length < 5  || weakSelf.room.sessionId.length < 5) {
                                               [weakSelf statusMessage:@77];
                                               return;
                                           }
                                           weakSelf.TacoHandler.kToken = weakSelf.room.token;
                                           weakSelf.TacoHandler.kSessionId = weakSelf.room.sessionId;
                                           [weakSelf.TacoHandler doConnectToRoomWithSession];
                                       } failure: ^(RKObjectRequestOperation *operation, NSError *error) {
                                           //Retry?
                                           [weakSelf startRetryTimer];
                                           NSLog(@"failed");
                                       }];
}

- (void)dealloc {
    [[RKObjectManager sharedManager].operationQueue cancelAllOperations];
    self.TacoHandler = nil;
    self.room = nil;
    NSLog(@"*******deallocated**********");
}

- (void)doCloseRoom {
    if (self.room.roomId.intValue < 1) {
        return;
    }
    __weak typeof(self) weakSelf = self;

    [[RKObjectManager sharedManager] deleteObject:weakSelf.room
                                             path:nil
                                       parameters:nil
                                          success:nil
                                          failure:nil
     ];
}

#pragma mark - label
- (void)updateStatusLabel:(NSString *)message withColor:(UIColor *)color animated:(bool)animated {
    self.statusLabel.text = message;
    if (animated) {
        [self fadeOutFadeInAnimation:self.statusLabel andColor:color];
    }
    else {
        [SRAnimationHelper stopAnimations:self.statusLabel];
    }
}

- (void)fadeOutFadeInAnimation:(UILabel *)label andColor:(UIColor *)color {
    //add animation
    [label.layer addAnimation:[SRAnimationHelper fadeOfRoomStatusLabel] forKey:nil];

    //change label color
    label.textColor = color;
}

SRDetailViewController.h

#import <UIKit/UIKit.h>
#import "SRRoom.h"
#import "SRAPI.h"
#import "SRTacoVideoHandler.h"
#import "SRSocialSharing.h"
#import "SRAnimationHelper.h"


@interface SRDetailViewController : UIViewController

@property (weak, nonatomic) IBOutlet UILabel *roomTitle;
@property (weak, nonatomic) IBOutlet UIView *userScreenContainer;
@property (weak, nonatomic) IBOutlet UIView *opponentScreenContainer;
@property (weak, nonatomic) IBOutlet UILabel *statusLabel;
@property (weak, nonatomic) IBOutlet UIButton *retryButton;

@property (weak, nonatomic) IBOutlet UIProgressView *progressBar;
@property (strong, nonatomic) NSTimer *progressTimer;
@property (strong, nonatomic) NSTimer *retryTimer;
@property (weak, nonatomic) IBOutlet UIView *bottomViewContainer;


@property (strong, nonatomic) SRRoom *room;

@property (strong, nonatomic) SRTacoVideoHandler *TacoHandler;


@end

SRDetailViewController.m

#import "SRDetailViewController.h"
#import <QuartzCore/QuartzCore.h>

@interface SRDetailViewController ()

@property (strong, nonatomic) NSString *kApiKey;
@property (strong, nonatomic) NSString *kSessionId;
@property (strong, nonatomic) NSString *kToken;


@end

@implementation SRDetailViewController


- (void)viewDidLoad {
    [super viewDidLoad];
    [self configOpentTok];
    [self performGetRoomRequest];
    [self configNavBar];
    [self configNotifcations];
    [self configProgressBar];
}

- (void)configSocialSharing {
    //check if it already exists
    for (UIView *subview in self.view.subviews) {
        if ([subview isKindOfClass:[SRSocialSharing class]]) {
            return;
        }
    }

    //add off screen
    CGRect frame = CGRectMake(0, [[UIScreen mainScreen] bounds].size.height, [[UIScreen mainScreen] bounds].size.width, 44);
    SRSocialSharing *share = [[SRSocialSharing alloc] initWithFrame:frame];
    [self.view addSubview:share];

    share.sharingURL = [self createUrlForSharing];
    //animate in
    frame = CGRectMake(0, [[UIScreen mainScreen] bounds].size.height - 100, [[UIScreen mainScreen] bounds].size.width, 44);
    [UIView animateWithDuration:3 delay:2 options:UIViewAnimationOptionCurveEaseOut animations: ^{
        share.frame = frame;
    } completion:nil];
}

- (NSURL *)createUrlForSharing {
    NSString *urlString = [NSString stringWithFormat:@"MyUrl/room/%@/%@?session=%@", self.room.topicId, [self opposingPosition:self.room.position], self.room.sessionId];
    NSURL *url = [NSURL URLWithString:urlString];
    return url;
}

- (NSString *)opposingPosition:(NSString *)position {
    return ([position isEqualToString:@"agree"]) ? @"disagree" : @"agree";
}

- (void)configOpentTok {
    [self.tacoHandler registerUserVideoStreamContainer:self.userScreenContainer];
    self.tacoHandler.userVideoStreamConatinerName = self.room.position;

    [self.tacoHandler registerOpponentOneVideoStreamContainer:self.opponentScreenContainer];
    self.tacoHandler.opponentOneVideoStreamConatinerName = [self opposingPosition:self.room.position];

    self.TacoHandler.shouldPublish = YES;
    self.TacoHandler.isObserving = NO;
}

- (void)configNavBar {
    UIImage *backButtonImage = [UIImage imageNamed:@"backButton"];
    UIButton *backButton = [UIButton buttonWithType:UIButtonTypeCustom];
    [backButton setFrame:CGRectMake(0, 0, 47, 32)];
    [backButton setImage:backButtonImage forState:UIControlStateNormal];
    [backButton addTarget:self action:@selector(pressBackButton) forControlEvents:UIControlEventTouchUpInside];
    UIBarButtonItem *navBackButton = [[UIBarButtonItem alloc] initWithCustomView:backButton];

    [self.navigationItem setLeftBarButtonItem:navBackButton];

    self.title = [self.room.position stringByReplacingCharactersInRange:NSMakeRange(0, 1)
                                                             withString:[[self.room.position substringToIndex:1] capitalizedString]];
}

- (void)pressBackButton {
    self.navigationItem.leftBarButtonItem.enabled = NO;
    [self manageSafeClose];
    [self stopTimer:self.retryTimer];
    [self stopTimer:self.progressTimer];

    [self.TacoHandler safetlyCloseSession];
    double delayInSeconds = 3;
    //[self updateStatusLabel:@"Disconnecting" withColor:[UIColor grayColor]];

    dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delayInSeconds * NSEC_PER_SEC));
    dispatch_after(popTime, dispatch_get_main_queue(), ^(void) {
        [self.navigationController popViewControllerAnimated:YES];
    });
}

- (void)configNotifcations {
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(recieveNotifications:)
                                                 name:kSRTacoVideoHandlerNotifcations
                                               object:nil
     ];
}

- (void)recieveNotifications:(NSNotification *)notification {
    if ([[notification name] isEqualToString:kSRTacoVideoHandlerNotifcations]) {
        NSDictionary *userInfo = notification.userInfo;
        NSNumber *message = [userInfo objectForKey:@"message"];
        [self statusMessage:message];
    }
}

- (void)statusMessage:(NSNumber *)message {
    NSString *result = nil;

    switch ([message intValue]) {
        case 0:
            result = @"Disconnected";
            break;

        case 1:
            result = @"Connecting...";
            [self startRetryTimer];
            break;

        case 2:
            result = @"Publishing Your Video...";
            break;

        case 3:
            result = @"Searching for Idiots...";
            break;

        case 4:
            result = @"Start Shouting!";
            [self startProgressBar];
            [self stopTimer:self.retryTimer];
            break;

        case 5:
            [self stopTimer:self.progressTimer];
            result = @"Opponent Stopped Shouting! You Win!";
            break;

        case 6:
            [self stopTimer:self.progressTimer];
            result = @"Disconnecting...";
            break;

        case 7:
            result = @"Opponent failed to join. Retrying...";
            [self performSelector:@selector(retry) withObject:nil afterDelay:4];
            break;

        default:
            result = @"Retry";
    }

    [self updateStatusLabel:result withColor:[self statusLabelColorPicker:message] animated:YES];
    NSLog(@"STATUS LABEL UPDATE: %@", message);
}

- (UIColor *)statusLabelColorPicker:(NSString *)Message {
    return [UIColor whiteColor];
}

- (void)performGetRoomRequest {
    [[RKObjectManager sharedManager] getObject:self.room
                                          path:nil
                                    parameters:nil
                                       success: ^(RKObjectRequestOperation *operation, RKMappingResult *mappingResult) {
                                           self.TacoHandler.kToken = self.room.token;
                                           self.TacoHandler.kSessionId = self.room.sessionId;
                                           self.roomTitle.text = self.room.title;
                                           self.navigationController.title = self.room.position;
                                           [self configSocialSharing];
                                           [self.TacoHandler doConnectToRoomWithSession];
                                       } failure: ^(RKObjectRequestOperation *operation, NSError *error) {
                                           //Retry?
                                       }];
}

- (void)dealloc {
    [self stopTimer:self.retryTimer];
    [self stopTimer:self.progressTimer];
    [[RKObjectManager sharedManager].operationQueue cancelAllOperations];
    self.TacoHandler = nil;
    self.room = nil;
    [[NSNotificationCenter defaultCenter] removeObserver:self];
    NSLog(@"*******deallocated**********");
}

- (void)manageSafeClose {
    [self doCloseRoom];
}

- (void)doCloseRoom {
    [[RKObjectManager sharedManager] deleteObject:self.room
                                             path:nil
                                       parameters:nil
                                          success:nil
                                          failure:nil
     ];
}

- (void)startRetryTimer {
    NSLog(@"Timer Started");
    self.retryTimer  = [NSTimer scheduledTimerWithTimeInterval:(60 * 5)
                                                        target:self
                                                      selector:@selector(retry)
                                                      userInfo:nil
                                                       repeats:YES];
}

- (void)retry {
    [self doCloseRoom];
    [self stopTimer:self.progressTimer];
    [self stopTimer:self.progressTimer];
    [self.TacoHandler safetlyCloseSession];
    [self performSelector:@selector(performGetRoomRequest) withObject:nil afterDelay:4];
}

#pragma mark - label
- (void)updateStatusLabel:(NSString *)message withColor:(UIColor *)color animated:(bool)animated {
    self.statusLabel.text = message;
    if (animated) {
        [self fadeOutFadeInAnimation:self.statusLabel andColor:color];
    }
    else {
        [SRAnimationHelper stopAnimations:self.statusLabel];
    }
}

- (void)fadeOutFadeInAnimation:(UILabel *)label andColor:(UIColor *)color {
    //add animation
    [label.layer addAnimation:[SRAnimationHelper fadeOfRoomStatusLabel] forKey:nil];

    //change label color
    label.textColor = color;
}

#pragma mark - Progress Bar
- (void)configProgressBar {
    self.progressBar.progressTintColor = [UIColor orangeColor];
}

- (void)startProgressBar {
    self.progressBar.hidden = NO;
    self.progressBar.progress = 0;
    self.progressTimer  = [NSTimer scheduledTimerWithTimeInterval:.5
                                                           target:self
                                                         selector:@selector(changeProgressValue)
                                                         userInfo:nil
                                                          repeats:YES];
}

- (void)stopTimer:(NSTimer *)timer {
    [timer invalidate];
    timer = nil;
}

- (void)changeProgressValue {
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        float progressValue         = self.progressBar.progress;
        progressValue               += .00834;
        if (progressValue > .99) {
            progressValue = 1;
            [self stopTimer:self.progressTimer];
            return;
        }

        NSString *time = [NSString stringWithFormat:@"%.0f", 60 - ceil(progressValue * 60)];
        NSLog(@"Progress Value %f Time %@", progressValue, time);


        NSString *message = [NSString stringWithFormat:@"Time Left: %@",  time];

        dispatch_async(dispatch_get_main_queue(), ^(void) {
            self.progressBar.progress      = progressValue;
            [self updateStatusLabel:message withColor:[UIColor whiteColor] animated:NO];
        });
    });
}

@end

Answer 1

From your description your approach with two different ViewControllers seems about right. It might make sense to base them of another baseClass, but I find that too busy base-classes often reduce readability at the altar of reusing code. I guess this part boils down to your personal preferences and the complexity of your project.

One part that do seems ripe for improvements is the statusMessage methods in both classes. First of the name indicates that they return a value, displayStatusMessageForNumber (or similar) makes more sense.

Also using a dictionary, matching the numbers and messages would allow a lot less logic inside each switch-statement. As almost every single option either calls stopRetryTimer or startRetryTimer this will simplify things quite a lot. Finally using an enum for the status-codes would make the statement a lot more readable to people without detailed knowledge about each code...

NS_ENUM(NSInteger, statusKey){
    SRStatusKeyDisconnected = 0,
    SRStatusKeyConnecting = 1,
    SRStatusKeySearchingForIdiots = 3,
    SRStatusKeyWowLookAtThatGuy = 4,
    SRStatusKeyMatchOver = 5,
    SRStatusKeyDisconnecting = 6,
    SRStatusKeyOpponentFailed = 7,
    SRStatusKeySearchSucceededNoRooms = 77,
    SRStatusKeyObservingSpecimens = 88,
    SRStatusKeyEveryoneLeftWillRestart = 99
};

...

 - (NSDictionary *)statusKeysAndMessages{
    NSDictionary *statusKeysAndMessages = @{ @0 : @"Disconnected",
                                             @1 : @"Connecting...",
                                             @3 : @"Searching for Idiots...",
                                             @4 : @"Wow, look at that guy...",
                                             @5 : @"Match Over! Will start searching for new room shortly...",
                                             @6 : @"Disconnecting...",
                                             @7 : @"Opponent failed to Join. Will start searching for new room shortly...",
                                             @77 : @"Searching for Idiots...",
                                             @88 : @"Observing specimens" ,
                                             @99 : @"Everyone Left! Will start searching for new room shortly..."};
    return statusKeysAndMessages;
}

...

- (void)displayStatusMessageForNumber:(NSNumber *)message {
    NSString *result = [[self statusKeysAndMessages] objectForKey:message];

    switch ([message intValue]) {
        case SRStatusKeyDisconnected:
        case SRStatusKeySearchingForIdiots:
        case SRStatusKeyOpponentFailed:
        case SRStatusKeySearchSucceededNoRooms:
            [self startRetryTimer];
            break;
        case SRStatusKeyConnecting:
        case SRStatusKeyWowLookAtThatGuy:
        case SRStatusKeyDisconnecting:
        case SRStatusKeyObservingSpecimens:
            [self stopRetryTimer];
            break;
        case SRStatusKeyMatchOver:
        case SRStatusKeyEveryoneLeftWillRestart:
            [self stopRetryTimer];
            [self performSelector:@selector(retry) withObject:nil afterDelay:6];
            break;
        default:
            break;
    }

    if (!result) {
        result = @"Retry";
    }

    [self updateStatusLabel:result withColor:[self statusLabelColorPicker:message] animated:YES];
    NSLog(@"STATUS LABEL UPDATE: %@", message);
}

From what I can see the code handling the status messages in both viewControllers have a lot in common. Perhaps this might warrant creating an own class handling the messages. The more I look at you code, the more this makes sense. As the status messages are mostly(?) married to the video stuff and is handled through a notification pattern you could also move this part of the code into such a handler-class...