2
\$\begingroup\$

I'd like to see how to improve this code. I have a UITableView that I'm 'hiding' a section when the user taps on that section's header. I thought this was a pretty cool implementation I came up with, but it turns out when this section is hidden and the user pulls to refresh, about 1-2MB is added into memory every time they pull to refresh. If the section isn't hidden and they pull to refresh, no extra memory is allocated (the correct way/behavior).

I may be overlooking something, and would appreciate any help offered. Sorry for the lengthy code below:

Update: added cellForRowAtIndexPath method below

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    UITableViewCell *cell;
    NSString *identifier = @"cellIdentifier";
    UILabel *userNameLabel = [self userNameLabel];
    UIView *requestsView = [self requestsView];
    UIButton *approveButton = [self approveButton];
    UIButton *denyButton = [self denyButton];
    UIImageView *profileImageView = [self profileImageView];

    if (cell == nil)
    {
        cell = [[UITableViewCell alloc]initWithStyle:UITableViewCellStyleDefault reuseIdentifier:identifier];
        cell.selectionStyle = UITableViewCellSelectionStyleDefault;

        [requestsView addSubview:approveButton];
        [requestsView addSubview:denyButton];

        [cell.contentView addSubview:requestsView];
        [cell.contentView addSubview:profileImageView];
        [cell.contentView addSubview:userNameLabel];
    }

    if (indexPath.section == 0)
    {
        cell.selectionStyle = UITableViewCellSelectionStyleNone;
        userNameLabel.frame = CGRectMake(82, 0, 167, 68);
        approveButton.tag = indexPath.row;
        denyButton.tag = indexPath.row;

        NSArray *keys = [self.tableDataSource objectForKey:@"Requests"];
        id aKey = [keys objectAtIndex:indexPath.row];

        userNameLabel.text = [aKey objectForKey:@"User"];

        NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:[aKey objectForKey:@"URL"]]];
        request.cachePolicy = NSURLRequestReloadIgnoringLocalAndRemoteCacheData;
        request.timeoutInterval = 5.0;

        if ([imageCache objectForKey:aKey])
        {
            profileImageView.image = [imageCache objectForKey:aKey];
        }
        else
        {
            [NSURLConnection sendAsynchronousRequest:request
                                               queue:[NSOperationQueue mainQueue]
                                   completionHandler:
             ^(NSURLResponse *response, NSData *data, NSError *error)
             {
                 if (data)
                 {
                     dispatch_async(dispatch_get_main_queue(), ^
                    {
                        profileImageView.image = [UIImage imageWithData:data];
                        [imageCache setObject:[UIImage imageWithData:data] forKey:aKey];
                    });
                 }
                 else
                 {
                     profileImageView.image = [self profileImage];
                 }
             }];
        }
    }
    else if (indexPath.section == 1)
    {
        requestsView.hidden = YES;
        NSArray *keys = [self.tableDataSource objectForKey:@"Friends"];
        id aKey = [keys objectAtIndex:indexPath.row];
        userNameLabel.text = [aKey objectForKey:@"User"];

        NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:[aKey objectForKey:@"URL"]]];
        request.cachePolicy = NSURLRequestReloadIgnoringLocalAndRemoteCacheData;
        request.timeoutInterval = 5.0;

        if ([imageCache objectForKey:aKey])
        {
            profileImageView.image = [imageCache objectForKey:aKey];
        }
        else
        {
            [NSURLConnection sendAsynchronousRequest:request
                                               queue:[NSOperationQueue mainQueue]
                                   completionHandler:
             ^(NSURLResponse *response, NSData *data, NSError *error)
             {
                 if (data)
                 {
                     dispatch_async(dispatch_get_main_queue(), ^
                    {
                        profileImageView.image = [UIImage imageWithData:data];
                        [imageCache setObject:[UIImage imageWithData:data] forKey:aKey];
                    });
                 }
                 else
                 {
                     profileImageView.image = [self profileImage];
                 }
             }];
        }
    }
    else if (indexPath.section == 2)
    {
        requestsView.hidden = YES;
        NSArray *keys = [self.tableDataSource objectForKey:@"Blocked"];
        id aKey = [keys objectAtIndex:indexPath.row];
        userNameLabel.text = [aKey objectForKey:@"User"];
    }

    return cell;
}

- (void)displayBlockedUsers
{
    if (self.isShowingBlocked == YES)
    {
        self.isShowingBlocked = NO;
        [self.tableView reloadSections:[NSIndexSet indexSetWithIndex:2] withRowAnimation:UITableViewRowAnimationNone];
    }
    else
    {
        self.isShowingBlocked = YES;
        [self.tableView reloadSections:[NSIndexSet indexSetWithIndex:2] withRowAnimation:UITableViewRowAnimationNone];
    }
}

- (NSInteger)friendRequests
{
    return [[self.tableDataSource objectForKey:@"Requests"]count];
}

- (NSInteger)friends
{
    return [[self.tableDataSource objectForKey:@"Friends"]count];
}

- (NSInteger)blockedUsers
{
    return [[self.tableDataSource objectForKey:@"Blocked"]count];
}

- (CGFloat)tableView:(UITableView *)tableView heightForRowAtIndexPath:(NSIndexPath *)indexPath
{
    if (indexPath.section == 2 && self.isShowingBlocked == NO)
    {
        return 0;
    }
    else
    {
       return 68;
    }
}

- (CGFloat)tableView:(UITableView *)tableView heightForHeaderInSection:(NSInteger)section
{
    if (section == 0)
    {
        if ([self friendRequests] < 1)
        {
            return 0;
        }
        else
        {
            return 22;
        }
    }
    else if (section == 1)
    {
        if ([self friends] < 1)
        {
            return 0;
        }
        else
        {
            return 22;
        }
    }
    else
    {
        if ([self blockedUsers] < 1)
        {
            return 0;
        }
        else
        {
            return 22;
        }
    }
}

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    if (section == 0)
    {
        if ([self friendRequests] < 1)
        {
            return 0;
        }
        else
        {
            return [self friendRequests];
        }
    }
    else if (section == 1)
    {
        if ([self friends] < 1)
        {
            return 0;
        }
        else
        {
            return [self friends];
        }
    }
    else
    {
        if ([self blockedUsers] < 1)
        {
            return 0;
        }
        else
        {
            return [self blockedUsers];
        }
    }
}

- (UIView *)tableView:(UITableView *)tableView viewForHeaderInSection:(NSInteger)section
{
    UIView *headerView = [[UIView alloc]init];
    headerView.backgroundColor = [UIColor colorWithRed:248.0/255.0 green:248.0/255.0 blue:248.0/255.0 alpha:1.0];

    UILabel *headerLabel = [[UILabel alloc]initWithFrame:CGRectMake(12, 0, 320, 22)];
    headerLabel.font = [UIFont systemFontOfSize:13.5];
    [headerView addSubview:headerLabel];

    if (section == 0)
    {
        if ([self friendRequests] < 2)
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Request",[self friendRequests]];
        }
        else
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Requests",[self friendRequests]];
        }
    }
    else if (section == 1)
    {
        if ([self friends] < 2)
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Friend",[self friends]];
        }
        else
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Friends",[self friends]];
        }
    }
    else
    {
        UIButton *theButton = [UIButton buttonWithType:UIButtonTypeCustom];
        [theButton addTarget:self action:@selector(displayBlockedUsers) forControlEvents:UIControlEventTouchUpInside];
        theButton.frame = CGRectMake(0, 0, 320, 22);

        [headerView addSubview:theButton];

        if (self.isShowingBlocked == YES)
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Blocked - Tap to Hide",[self blockedUsers]];
        }
        else
        {
            headerLabel.text = [NSString stringWithFormat:@"%d Blocked - Tap to Show",[self blockedUsers]];
        }
    }

    return  headerView;
}

- (CGFloat)tableView:(UITableView *)tableView heightForFooterInSection:(NSInteger)section
{
   return 0;
}

- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView
{
    return 3;
}
\$\endgroup\$
5
\$\begingroup\$

The memory leak is more on topic for StackOverflow, but while this question is here and open, I'll make some comments on the stuff that's on topic...

Your - (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section method really, really bothers me.

It can be replaced with this:

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {
    switch(section) {
        case 0:
            return [self friendRequests];
        case 1:
            return [self friends];
        default:
            return [self blockedUsers];
    }
}

These 3 methods will all return a value that is either 0 or evaluates false in < 1. So the only time < 1 evaluates true (and you want to return 0) is when the method itself is returning 0 (these methods can't return a negative number).


As for the memory issue, you'd need to include the cellForRowAtIndexPath: method.


Your code's readability could also be greatly enhanced by adding the following enum:

typedef NS_OPTIONS(NSInteger FriendTableSections) {
    FriendRequests = 0,
    Friends        = 1,
    BlockedUsers   = 2
};

And then referring to the sections by their name in this enum, rather than by simply "0", "1", "2" (or "else").

\$\endgroup\$
  • \$\begingroup\$ Wow, thank you for the valid help with the switch statement. My previous method that I was doing definitely is not as light weight as the one you kindly provided. Also I will provide the cellForRowAtIndexPath method in a few \$\endgroup\$ – klcjr89 Feb 21 '14 at 23:52
  • \$\begingroup\$ Just added the code in original question above \$\endgroup\$ – klcjr89 Feb 21 '14 at 23:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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