patternMinor
Filling text boxes with player names received from an array
Viewed 0 times
boxeswithplayertextarraynamesfillingreceivedfrom
Problem
I'm currently trying to make the code of an app I'm developing a bit more efficient and easier to read. Basically what this does is after the user taps with two fingers (gesture) it retrieves a previously saved array from
I'm trying to reduce the length of a long
```
if (gestureRecognizer.state == UIGestureRecognizerStateEnded) {
NSMutableArray *names = [[NSMutableArray alloc] initWithArray:[[NSUserDefaults standardUserDefaults] objectForKey:@"nameArray"]];
for (int i = 0; i <= 5; i++) {
NSIndexPath *indexPath = [NSIndexPath indexPathForRow:i inSection:0];
UITableViewCell *cell = [playerTable cellForRowAtIndexPath:indexPath];
for (UIView *view in cell.contentView.subviews) {
if ([view isKindOfClass:[UITextField class]]) {
UITextField txtField = (UITextField )view;
if (txtField.tag == 6) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text = [names objectAtIndex:0]; }
else {
txtField.text = @"Peter";
}
}
if (txtField.tag == 7) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text = [names objectAtIndex:1]; }
else {
txtField.text = @"Julia";
}
}
if (txtField.tag == 8) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text
NSUserDefaults of player names, and fills in the 6 text boxes (tagged 6-11) on a table view with these names. If there isn't an existing array it'll use a default set of names.I'm trying to reduce the length of a long
for/if statement. Any ideas for simplifying this code would be appreciated.```
if (gestureRecognizer.state == UIGestureRecognizerStateEnded) {
NSMutableArray *names = [[NSMutableArray alloc] initWithArray:[[NSUserDefaults standardUserDefaults] objectForKey:@"nameArray"]];
for (int i = 0; i <= 5; i++) {
NSIndexPath *indexPath = [NSIndexPath indexPathForRow:i inSection:0];
UITableViewCell *cell = [playerTable cellForRowAtIndexPath:indexPath];
for (UIView *view in cell.contentView.subviews) {
if ([view isKindOfClass:[UITextField class]]) {
UITextField txtField = (UITextField )view;
if (txtField.tag == 6) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text = [names objectAtIndex:0]; }
else {
txtField.text = @"Peter";
}
}
if (txtField.tag == 7) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text = [names objectAtIndex:1]; }
else {
txtField.text = @"Julia";
}
}
if (txtField.tag == 8) {
if([[NSUserDefaults standardUserDefaults] boolForKey:@"customNames"]) {
txtField.text
Solution
We can do better still.
First of all, we're talking about a
What makes most sense is to instead subclass
Then, you would no longer need to iterate through all of the content view's subviews. Instead, best case scenario, you only have a single text field on the cell, and you can access it directly, and
This chunk of code is confusing.
First of all,
Second of all, presuming the
I think there's some more cleanup to do, but I'm kind of interested in seeing whether you'd be interested in posting a new question where you've rewritten the code to use a
First of all, we're talking about a
UITableViewCell to which you've added custom views--or at least modified existing views (hence the tags) as well as adding a gesture recognizer.What makes most sense is to instead subclass
UITableViewCell. This would help both to clean up the method you have posted here as well as clean up the cellForRowAtIndexPath: which I'm sure is a bit of a mess.Then, you would no longer need to iterate through all of the content view's subviews. Instead, best case scenario, you only have a single text field on the cell, and you can access it directly, and
switch on its tag, and at worst, you can create a custom IBCollection--an array of text fields--that would be relevant to this method and iterate over just that (which you know would contain only text fields relevant to this method) looking at the tags.int index = [txtField tag]-offsetIndex;
if (tag >= 6 && tag <= 11)
{
if (needCustomNames)
textField.text = [names objectAtIndex:index];
else
textField.text = [defaultNames objectAtIndex:index];
}This chunk of code is confusing.
First of all,
tag isn't declared anywhere.Second of all, presuming the
if is supposed to be checking textField.tag, you should either move the int index line inside the if block, or probably better, refactor the if to check the index that was just calculated based on textField.tag and is the value actually used inside the if block. As it stands, this code might crash if someone unknowingly changes the offsetIndex 7 lines previously and that might be a hard crash to figure out.I think there's some more cleanup to do, but I'm kind of interested in seeing whether you'd be interested in posting a new question where you've rewritten the code to use a
UITableViewCell subclass, which truly would be the best option.Code Snippets
int index = [txtField tag]-offsetIndex;
if (tag >= 6 && tag <= 11)
{
if (needCustomNames)
textField.text = [names objectAtIndex:index];
else
textField.text = [defaultNames objectAtIndex:index];
}Context
StackExchange Code Review Q#56632, answer score: 6
Revisions (0)
No revisions yet.