patternMinor
iOS App ViewController
Viewed 0 times
viewcontrollerappios
Problem
How can I most effectively refactor the code in these 2
I'm familiar with subclassing in Objective-C and have used it extensively else where with
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
Edit:
Overview & Screenshot
SRObserveViewController.h:
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
[super viewDidLoad];
[self configOpentTok];
[self performGetRoomRequest];
[self configNavBar];
[s
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
SRRoomwhere 2 people are already matched up and can watch them talk.
TacoVideoHandler abstracts all the nitty gritty video methods.SRObserveViewController.h:
#import
#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;
@endSRObserveViewController.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];
[s
Solution
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...
...
...
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...
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...
Code Snippets
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);
}Context
StackExchange Code Review Q#32509, answer score: 5
Revisions (0)
No revisions yet.