iosobjective-cnsoperation

NSOperation class run multiple operations


So I've asked a couple of questions regarding the UICollectionView. Understanding how it works, I'm trying to implement lazy loading to load 15 images onto the view controller. I found many examples 1, 2, 3...first and third examples deal with only one operation, second example I don't think uses operations at all, only threads. My question is would it be possible to use a NSOperation class and use/reuse operations? I read that you can't rerun operations but I think you are able to once you initialize them again. Here's my code:

view controller:

UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc]init];
layout.sectionInset = UIEdgeInsetsMake(10, 20, 10, 20);
[layout setItemSize:CGSizeMake(75, 75)];
self.images = [[UICollectionView alloc]initWithFrame:CGRectMake(0, 230, self.view.frame.size.width, 200) collectionViewLayout:layout];
self.images.delegate = self;
self.images.dataSource = self;
[self.images registerClass:[UICollectionViewCell class] forCellWithReuseIdentifier:@"cellIdentifier"];
[self.view addSubview:self.images];
self.operation = [[NSOperationQueue alloc]init];
[self.operation addOperationWithBlock:^{
    NSURL *url = [NSURL URLWithString:[NSString stringWithFormat:@"getgallery.php?user=%@", userId] relativeToURL:[NSURL URLWithString:@"http://www.mywebsite.com/"]];
    NSData *data = [NSData dataWithContentsOfURL:url];
//datasource for all images
    self.imagesGalleryPaths = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingMutableContainers error:nil];
    [[NSOperationQueue mainQueue]addOperationWithBlock:^{
        [self.images reloadData];
//reload collection view to place placeholders
    }];
}];

- (void)viewDidAppear:(BOOL)animated{
//get the visible cells right away
visibleCellPaths = [NSArray new];
visibleCellPaths = self.images.indexPathsForVisibleItems;
self.processedImages = [[NSMutableDictionary alloc]initWithCapacity:visibleCellPaths.count];
}
#pragma mark - collection view
- (NSInteger)numberOfSectionsInCollectionView:(UICollectionView *)collectionView{
return 1;
}

- (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSection:(NSInteger)section{
int i;
if (self.imagesGalleryPaths.count == 0)
    i = 25;
else
    i = self.imagesGalleryPaths.count;
return i;
}

- (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath{
UICollectionViewCell *cell = [UICollectionViewCell new];
cell = [collectionView dequeueReusableCellWithReuseIdentifier:@"cellIdentifier" forIndexPath:indexPath];
cell.layer.borderWidth = 1;
cell.layer.borderColor = [[UIColor whiteColor]CGColor];
UIImageView *image = [[UIImageView alloc]init];
if (self.imagesGalleryPaths.count != 0) {
    if ([visibleCellPaths containsObject:indexPath]) {
        [self setUpDownloads:visibleCellPaths];
    }
    image.image = [UIImage imageNamed:@"default.png"];
    image.frame = CGRectMake(0, 0, cell.frame.size.width, cell.frame.size.height);
    [cell.contentView addSubview:image];
}
return cell;
}

- (void)scrollViewDidEndDragging:(UIScrollView *)scrollView willDecelerate:(BOOL)decelerate{
visibleCellPaths = [NSArray new];
visibleCellPaths = self.images.indexPathsForVisibleItems;
[self setUpDownloads:visibleCellPaths];
}

- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView{
visibleCellPaths = [NSArray new];
visibleCellPaths = self.images.indexPathsForVisibleItems;
[self setUpDownloads:visibleCellPaths];
}

- (void)setUpDownloads:(NSArray *)visiblePaths{
//I want to pass the visiblePaths to the NSOperation class if visible cells changed
GalleryOps *gallery = [[GalleryOps alloc]init];
//I will use a dictionary to keep track of which indexPaths are being downloaded...
//...so there are no duplicate downloads
}

GalleryOps.m

@implementation GalleryOps

- (void)main{
    //would I initialize all the operations here and perform them?
}

It's almost pointless to show the empty GalleryOps class because I have no idea how to initialize it with multiple operations. I know I have to override the main method, and once I get the image data from URL, I'll need to update the UI, for which I need a delegate method...another thing I don't yet know how to do but there are many examples to figure that out. My biggest question is how to pass the visible cells into this class and run multiple operations? When new visible cells come in, I'll run a check to see which to cancel, which to keep. Any advice here? Thanks in advance!


Solution

  • Looking at your proposed solution, it looks like you want to defer the question of making the operations cancelable. Furthermore, it looks like you want to defer the use of the cache (even though it's no more complicated than your NSMutableDictionary property).

    So, setting that aside, your revised code sample has two "big picture" issues:

    1. You can dramatically simplify the image retrieval process. The use of startOperationForVisibleCells and the two scroll delegates is unnecessarily complicated. There is a much simpler pattern in which you can retire those three methods (and achieve an even better UX).

    2. Your cellForItemForIndexPath has a problem, that you're adding subviews. The issue is that cells are reused, so every time a cell is reused, you're adding more redundant subviews.

      You really should subclass UICollectionViewCell (CustomCell in my example below), put the configuration of the cell, including the adding of subviews, there. It simplifies your cellForItemAtIndexPath and eliminates the problem of extra subviews being added.

    In addition to these two major issues, there were a bunch of little issues:

    1. You neglected to set maxConcurrentOperationCount for your operation queue. You really want to set that to 4 or 5 to avoid operation timeout errors.

    2. You are keying your imageGalleryData with the NSIndexPath. The problem is that if you ever deleted a row, all of your subsequent indexes would be wrong. I suspect this isn't an issue right now (you're probably not anticipating deleting of items), but if you keyed it by something else, such as the URL, it's just as easy, but it is more future-proof.

    3. I'd suggest renaming your operation queue from operation to queue. Likewise, I'd rename the UICollectionView from images (which might be incorrectly inferred to be an array of images) to something like collectionView. This is stylistic, and you don't have to do that if you don't want, but it's the convention I used below.

    4. Rather than saving the NSData in your NSMutableDictionary called imageGalleryData, you might want to save the UIImage instead. This saves you from having to reconvert from NSData to UIImage (which should make the scrolling process smoother) as you scroll back to previously downloaded cells.

    So, pulling that all together, you get something like:

    static NSString * const kCellIdentifier = @"CustomCellIdentifier";
    
    - (void)viewDidLoad
    {
        [super viewDidLoad];
    
        UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc]init];
        layout.sectionInset = UIEdgeInsetsMake(10, 20, 10, 20);
        [layout setItemSize:CGSizeMake(75, 75)];
    
        // renamed `images` collection view to `collectionView` to follow common conventions
    
        self.collectionView = [[UICollectionView alloc]initWithFrame:CGRectMake(0, 230, self.view.frame.size.width, 200) collectionViewLayout:layout];
        self.collectionView.delegate = self;
        self.collectionView.dataSource = self;
    
        // you didn't show where you instantiated this in your examples, but I'll do it here
    
        self.imageGalleryData = [NSMutableDictionary dictionary];
    
        // register a custom class, not `UICollectionViewCell`
    
        [self.collectionView registerClass:[CustomCell class] forCellWithReuseIdentifier:kCellIdentifier];
        [self.view addSubview:self.collectionView];
    
        // (a) change queue variable name;
        // (b) set maxConcurrentOperationCount to prevent timeouts
    
        self.queue = [[NSOperationQueue alloc]init];
        self.queue.maxConcurrentOperationCount = 5;
    
        [self.queue addOperationWithBlock:^{
            NSURL *url = [NSURL URLWithString:[NSString stringWithFormat:@"getgallery.php?user=%@", userId] relativeToURL:[NSURL URLWithString:@"http://www.mywebsite.com/"]];
            NSData *data = [NSData dataWithContentsOfURL:url];
            //datasource for all images
            self.imagesGalleryPaths = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingMutableContainers error:nil];
            [[NSOperationQueue mainQueue]addOperationWithBlock:^{
                [self.collectionView reloadData];
            }];
        }];
    }
    
    - (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSection:(NSInteger)section
    {
        return [self.imagesGalleryPaths count];          // just use whatever is the right value here, don't make this unnecessarily smaller
    }
    
    - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath
    {
        CustomCell *cell = [collectionView dequeueReusableCellWithReuseIdentifier:kCellIdentifier forIndexPath:indexPath];
    
        NSString *key = self.imagesGalleryPaths[indexPath.row];                 // I don't know whether this was simply array, or some nested structure, so tweak this accordingly
        UIImage *image = self.imageGalleryData[key]; 
    
        if (image) {
            cell.imageView.image = image;                                       // if we have image already, just use it
        } else {
            cell.imageView.image = [UIImage imageNamed:@"profile_default.png"]; // otherwise set the placeholder ...
    
            [self.queue addOperationWithBlock:^{                                // ... and initiate the asynchronous retrieval
                NSURL *url = [NSURL URLWithString:...];                         // build your URL from the `key` as appropriate
                NSData *responseData = [NSData dataWithContentsOfURL:url];
                if (responseData != nil) {
                    UIImage *downloadedImage = [UIImage imageWithData:responseData];
                    if (downloadedImage) {
                        [[NSOperationQueue mainQueue]addOperationWithBlock:^{
                            self.imageGalleryData[key] = downloadedImage;
                            CustomCell *updateCell = (id)[collectionView cellForItemAtIndexPath:indexPath];
                            if (updateCell) {
                                updateCell.imageView.image = downloadedImage;
                            }
                        }];
                    }
                }
            }];
        }
    
        return cell;
    }
    
    // don't forget to purge your gallery data if you run low in memory
    
    - (void)didReceiveMemoryWarning
    {
        [super didReceiveMemoryWarning];
        [self.imageGalleryData removeAllObjects];
    }
    

    Now, clearly I don't have access to your server, so I couldn't check this (notably, I don't know if your JSON is returning a full URL or just a filename, or whether there was some nested array of dictionaries). But I don't want to you to get too lost in the details of my code, but rather look at the basic pattern: Eliminate your looping through visible cells and responding to scroll events, and let cellForItemAtIndexPath do all the work for you.

    Now, the one thing that I introduced was the concept of CustomCell, which is a UICollectionViewCell subclass that might look like:

    //  CustomCell.h
    
    #import <UIKit/UIKit.h>
    
    @interface CustomCell : UICollectionViewCell
    
    @property (nonatomic, weak) UIImageView *imageView;
    
    @end
    

    and then move cell configuration and adding of the subview here to the @implementation:

    //  CustomCell.m
    
    #import "CustomCell.h"
    
    @implementation CustomCell
    
    - (id)initWithFrame:(CGRect)frame
    {
        self = [super initWithFrame:frame];
        if (self) {
            self.layer.borderWidth = 1;
            self.layer.borderColor = [[UIColor whiteColor]CGColor];
    
            UIImageView *imageView = [[UIImageView alloc] initWithFrame:CGRectMake(0, 0, frame.size.width, frame.size.height)];
            [self addSubview:imageView];
            _imageView = imageView;
        }
        return self;
    }
        
    @end
    

    By the way, I still contend that if you want to do this properly, you should refer to my other answer. And if you don't want to get lost in those weeds of the proper implementation, then just use a third party UIImageView category that supports asynchronous image retrieval, caching, prioritizing network requests of visible cells, etc., such as SDWebImage.