2
\$\begingroup\$

I'm using multiple instances of an object creating a form:

{...
BPFormInputTextFieldCell *nameCell = [[inputTextFieldClass alloc] init];
    nameCell.backgroundColor = cellBgnd;
    nameCell.textField.placeholder = @"business_name";
    nameCell.mandatory = YES;
    nameCell.textField.delegate = self;
    nameCell.customCellHeight = 40.0f;

    BPFormInputTextFieldCell *addressCell = [[inputTextFieldClass alloc] init];
    addressCell.backgroundColor = cellBgnd;
    addressCell.textField.placeholder = @"contact_name";
    addressCell.mandatory = YES;
    addressCell.textField.delegate = self;
    addressCell.customCellHeight = 40.0f;

    BPFormInputTextFieldCell *dobCell = [[inputTextFieldClass alloc] init];
    dobCell.backgroundColor = cellBgnd;
    dobCell.textField.placeholder = @"address";
    dobCell.mandatory = YES;
    dobCell.textField.delegate = self;
    dobCell.customCellHeight = 40.0f;

    BPFormInputTextFieldCell *sexCell = [[inputTextFieldClass alloc] init];
    sexCell.backgroundColor = cellBgnd;
    sexCell.textField.placeholder = @"city";
    sexCell.mandatory = YES;
    sexCell.textField.delegate = self;
    sexCell.customCellHeight = 40.0f;

    BPFormInputTextFieldCell *contexturaCell = [[inputTextFieldClass alloc] init];
    contexturaCell.backgroundColor = cellBgnd;
    contexturaCell.textField.placeholder = @"region";
    contexturaCell.mandatory = YES;
    contexturaCell.textField.delegate = self;
    contexturaCell.customCellHeight = 40.0f;

    self.formCells = @[@[nameCell, addressCell, dobCell, sexCell, contexturaCell, pesoCell, estaturaCell, actividadCell, deseoCell, cuantosKilosCell, cuantoTiempoCell, ...];
 ...}

As you can see, I need to refactor this badly. The objects should also go into the array. What is the best approach?

\$\endgroup\$
4
  • \$\begingroup\$ Is BPFormInputTextFieldCell your own subclass? Why not just move all of the redundant code to the initialization and pass in the textfield placeholder to the initializer? \$\endgroup\$
    – bazola
    Mar 27, 2015 at 18:56
  • \$\begingroup\$ hi @bazola, is not my class, is a cocoapods form generator... \$\endgroup\$ Mar 27, 2015 at 19:19
  • \$\begingroup\$ Can you subclass it? That will probably be the best approach \$\endgroup\$
    – bazola
    Mar 27, 2015 at 19:58
  • \$\begingroup\$ There are almost certainlly other issues with this code that are hidden behind the ... sections... but I'm posting an answer anyway. \$\endgroup\$
    – nhgrif
    Mar 27, 2015 at 21:37

2 Answers 2

3
\$\begingroup\$

I would subclass the Cell class and pass in arguments during initialization. The subclass would look something like this:

Interface

@interface MyBPFormInputTextFieldCell : BPFormInputTextFieldCell

+(MyBPFormInputTextFieldCell *) cellWithBackgroundColor:(UIColor *)color
                                                   text:(NSString *)text
                                              mandatory:(BOOL)isMandatory
                                               delegate:(Thing *)delegate
                                                 height:(CGFloat)height;

@end

Implementation

@implementation MyBPFormInputTextFieldCell

+(MyBPFormInputTextFieldCell *) cellWithBackgroundColor:(UIColor *)color
                                                   text:(NSString *)text
                                              mandatory:(BOOL)isMandatory
                                               delegate:(Thing *)delegate
                                                 height:(CGFloat)height {

    return [[MyBPFormInputTextFieldCell alloc]initWithBackgroundColor:color
                                                                 text:text
                                                            mandatory:isMandatory
                                                             delegate:delegate
                                                               height:height];
}

-(id) initWithBackgroundColor:(UIColor *)color text:(NSString *)text mandatory:(BOOL)isMandatory delegate:(Thing *)delegate height:(CGFloat)height {
    self = [super init];
    if (self) {
        this.backgroundColor = color;
        this.textField.placeholder = text;
        this.mandatory = isMandatory;
        this.textField.delegate = delegate;
        this.customCellHeight = height;
    }
    return self;
}

@end

Then your code would look more like this:

MyBPFormInputTextFieldCell *sexCell = [MyBPFormInputTextFieldCell cellWithBackgroundColor:cellBgnd text:@"city" mandatory:YES delegate:self height:40.f];

Or alternatively for increased readability:

MyBPFormInputTextFieldCell *sexCell = [MyBPFormInputTextFieldCell cellWithBackgroundColor:cellBgnd
                                                                                     text:@"city"
                                                                                mandatory:YES
                                                                                 delegate:self
                                                                                   height:40.f];

However I will say that if you cannot subclass the Cell class, then the way you are doing things is alright. UI code is often ugly even after you use methods like this to make it cleaner.

Finally, if most of your values are always going to be the same, then you would not need to pass in all of those arguments, you could just have the init method of your subclass automatically set all the properties (except the delegate and the text field).

\$\endgroup\$
4
\$\begingroup\$

As far as I can tell, the only difference between these objects is the placeholder text, right? I'm not sure how valuable it is that we have so many text fields and don't have a way of accessing them individually (to know what the user input in each... but nonetheless...).

So all we need is an array with the placeholder strings, and a for in loop.

NSArray *placeholders = @[@"business_name", @"contact_name", @"address", @"city", @"region"];

NSMutableArray *textFields = [NSMutableArray arrayWithCapacity:placeholders.count];

for (NSString *placeholder in placeholders) {
    BPFormInputTextFieldCell *textField = [[inputTextFieldClass alloc] init];
    textField.backgroundColor = cellBgnd;
    textField .textField.placeholder = placeholder;
    textField.mandatory = YES;
    textField.textField.delegate = self;
    textField.customCellHeight = 40.0f;
    [textFields addObject:textField];
}

self.formCells = textFields;

So much copy-and-pasting should almost always be a red flag for needing a loop.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.