UITableView cellForRowAtIndexPath

I'd like to see how I can improve this code, as I know it's bad practice to have cell reuse identifiers like this, but I could not find any other way to keep the cells that contain images from calling the server again instead of reading the cached image.

```

• (UITableViewCell )tableView:(UITableView )tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath

{
NSString *cellIdentifier = [NSString stringWithFormat:@"cell%d%d",indexPath.section,indexPath.row];

UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:cellIdentifier];

if (cell == nil)
{
UIImageView *profileImageView = [[UIImageView alloc]initWithFrame:CGRectMake(12, 4, 60, 60)];
profileImageView.image = [UIImage imageNamed:@"Profile"];
profileImageView.layer.cornerRadius = 30.0;
profileImageView.layer.masksToBounds = YES;
profileImageView.contentMode = UIViewContentModeScaleAspectFit;

UILabel *userNameLabel = [[UILabel alloc]init];
userNameLabel.frame = CGRectMake(82, 0, 228, 68);

cell = [[UITableViewCell alloc]initWithStyle:UITableViewCellStyleDefault reuseIdentifier:cellIdentifier];

[cell.contentView addSubview:profileImageView];
[cell.contentView addSubview:userNameLabel];
cell.selectionStyle = UITableViewCellSelectionStyleDefault;

if (indexPath.section == 0)
{
cell.selectionStyle = UITableViewCellSelectionStyleNone;

UIView *requestsView = [[UIView alloc]initWithFrame:CGRectMake(240, 0, 80, 68)];

UIButton *approveButton = [UIButton buttonWithType:UIButtonTypeSystem];

approveButton.frame = CGRectMake(0, 16, 36, 36);

[approveButton addTarget:self action:@selector(approveButtonChanged:) forControlEvents:UIControlEventTouchUpInside];

[approveButton setTintColor:[UIColor companyBlue]];

[approveButton setImage:[UIImage imageNamed:@"Approve"] forState:UIControlStateNormal];

So, first thing's first... we definitely need to break this down into groups of smaller methods which are all called from the cellForRowAtIndexPath:. The general rule of thumb is that you should be able to fit an entire method on your screen at the same time without the need to scroll in any direction... and I'm even stricter than that myself.

For example:

UIImageView *profileImageView = [[UIImageView alloc]initWithFrame:CGRectMake(12, 4, 60, 60)];
profileImageView.image = [UIImage imageNamed:@"Profile"];
profileImageView.layer.cornerRadius = 30.0;
profileImageView.layer.masksToBounds = YES;
profileImageView.contentMode = UIViewContentModeScaleAspectFit;

// ....

[cell.contentView addSubview:profileImageView];

We can improve this in many ways. First, add this method.

- (UIImageView *)profileImageView {
    profImgVw = [[UIImageView alloc] initWithFrame:
        CGRectMake(12.0f, 4.0f, 60.0f, 60.0f)];
    // we don't need to set the image if we're just going to change it later
    //profImgVw.image = [UIImage imageNamed:@"Profile"];
    profImgVw.layer.cornerRadius = 30.0f;
    profImgVw.layer.masksToBounds = YES;
    profImgVw.contentMode = UIViewContentModeScaleAspectFit;

    return profImgVw;
}

Now we can replace some lines in the cellForRowAtIndexPath: to look like this:

UIImageView *profileImageView = [self profileImageView];

// after this method call but before the following method call, you
// will need to set the image of the profileImageView.

[cell.contentView addSubview:profileImageView];

And we've already cut 5 lines out of your cellForRowAtIndexPath: in an attempt to get it down to a more manageable size in terms of readability and maintainability.

You should work through the rest of the method to make tweaks like this.

Now then, as to your specific question regarding how to keep around images instead of having to download them again every time the cell is displayed? My approach would be to store them in an NSMutableArray or perhaps an NSMutableDictionary. Whichever one you'd be more comfortable with in terms of pulling the data out. But here's some pseudo-code to get you started...

@property (nonatomic,strong) NSMutableArray *imgArray;

Create a @property in the .m file. Now, in our viewDidLoad or somewhere before we've started filling the table, we want to give the imgArray a capacity:

self.imgArray = [NSMutableArray arrayWithCapacity:someCapacity];

Where someCapacity is calculated in the same way you calculate the number of rows for the table view section 1.

Now then, in the cellForRowAtIndexPath: method when we're potentially downloading the images, we're going to do something a little similar to the approach I took in the example profileImageView method I showed you.

if (![self.imgArray objectAtIndex:indexPath.row]) {
    // we've never loaded the img before, so we need to download it
    // download it, then assign it to the array
    [self.imgArray insertObject:[UIImage imageWithData:data]
        atIndex:indexPath.row];
    profileImageView.image = [self.imgArray objectAtIndex:indexPath.row];
} else {
    // image is already in self.imgArray, so just grab it from there
    profileImageView.image = [self.imgArray objectAtIndex:indexPath.row];
}

Now we only need to keep the images in memory and we can let the table view recycle all the other elements of the array that don't need to be downloaded and can easily be redrawn on the fly.

StackExchange Code Review Q#41713, answer score: 5