HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Keeping UI code DRY in iOS projects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
keepingprojectsiosdrycode

Problem

I've subclassed UI button and used it in my View Controller's viewDidLoad method, like so:

PLOTDefaultButton *createAccountBtn = [[PLOTDefaultButton alloc] init];
CGRect createAccountBtnFrame = createAccountBtn.frame;
createAccountBtnFrame.origin.x = 20;
createAccountBtnFrame.origin.y = self.view.bounds.size.height - 135;
createAccountBtnFrame.size.width = self.view.bounds.size.width - 40;
createAccountBtn.frame = createAccountBtnFrame;
[createAccountBtn setTitle:@"Create an account" forState:UIControlStateNormal];

createAccountBtn.backgroundColor = [UIColor plotYellow];
createAccountBtn.titleLabel.textColor = [UIColor colorWithRed:0.165 green:0.212 blue:0.267 alpha:1];
createAccountBtn.layer.shadowColor = [UIColor colorWithRed:0.831 green:0.106 blue:0.082 alpha:1].CGColor;

[createAccountBtn addTarget:self action:@selector(showCreateAccount:) forControlEvents:UIControlEventTouchUpInside];
[self.view addSubview:createAccountBtn];

PLOTDefaultButton *loginBtn = [[PLOTDefaultButton alloc] init];
CGRect loginBtnFrame = loginBtn.frame;
loginBtnFrame.origin.x = 20;
loginBtnFrame.origin.y = self.view.bounds.size.height - 67.5;
loginBtnFrame.size.width = self.view.bounds.size.width - 40;
loginBtn.frame = loginBtnFrame;
[loginBtn setTitle:@"Log in" forState:UIControlStateNormal];
[loginBtn addTarget:self action:@selector(showLogin:) forControlEvents:UIControlEventTouchUpInside];
[self.view addSubview:loginBtn];


And the UIButton subclass:

```
  • (id)initWithFrame:(CGRect)frame


{
self = [super initWithFrame:frame];
if (self) {
self.backgroundColor = [UIColor plotDarkBlue];
self.titleLabel.textColor = [UIColor whiteColor];
self.layer.cornerRadius = 5;
self.layer.masksToBounds = NO;
self.layer.borderWidth = 0;

self.layer.shadowColor = [UIColor colorWithRed:0.086 green:0.110 blue:0.141 alpha:1].CGColor;
self.layer.shadowOpacity = 1;
self.layer.shadowRadius = 0;
self.layer.shadowOffset =

Solution

It doesn't necessarily bother me that so much of our styling code is outside of the init method (though I would change some of this up--I'll get back to that). What bothers me is that it seems it's all directly in viewDidLoad.

I don't know about you, but for me, there's regularly lots of things to set up in viewDidLoad. Here, you've committed 21 lines of viewDidLoad code to setting up these buttons. Once we add code for a few other UI elements, plus code for anything else we want to set up in viewDidLoad, the method very quickly becomes a mega-do-too-much-method.

What I'd rather see is a method to contain all the code that sets up the buttons, and let viewDidLoad call that method. I'd much rather see:

- (void)viewDidLoad {
    [super viewDidLoad];
    [self setupButtons];
    [self setupLabels];
    [self checkWebForSomething];
}


We've got who knows how many very distinct tasks that all want to happen in viewDidLoad (or any other view controller life-cycle event).

If we separate these tasks out into distinct methods, it makes maintenance easier in the future. I don't want to spend time skimming through the UI code when I really just need to fix something in the code that does the checkWebForSomething stuff (and vice versa).

Now, with all that said, let's talk about the specifics of your initWithFrame: method, shall we?

I don't mind that we set things like our corner radius and shadow off set in init. I don't even necessarily mind that we set our font and have a hard coded height for the button. The most bothersome of these 4 things to me would be the hard coded height, but there's plenty of Apple precedent for UI elements with fixed height or width.

What bothers me are the preset colors.

Honestly, I think rather than initWithFrame:, we should be seeing a method that looks more like:

- (UIButton *)initWithFrame:(CGRect)frame
                      title:(NSString *)title
            foregroundColor:(UIColor *)foregroundColor
            backgroundColor:(UIColor *)backgroundColor
                shadowColor:(UIColor *)shadowColor;

Code Snippets

- (void)viewDidLoad {
    [super viewDidLoad];
    [self setupButtons];
    [self setupLabels];
    [self checkWebForSomething];
}
- (UIButton *)initWithFrame:(CGRect)frame
                      title:(NSString *)title
            foregroundColor:(UIColor *)foregroundColor
            backgroundColor:(UIColor *)backgroundColor
                shadowColor:(UIColor *)shadowColor;

Context

StackExchange Code Review Q#61764, answer score: 4

Revisions (0)

No revisions yet.