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.

enter image description here

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