5

I've identified a simple edge case in UICollectionView's batchUpdate operations which should work but fails with

attempt to perform an insert and a move to the same index path ( {length = 2, path = 0 - 2})

My operation is to go from [A, B] --> [C, B', A]. This is done with updates:

  • A move from index 0 to 2
  • A reload of index 1
  • An insert at index 0

Clearly the error is wrong, the insert index is different from the move TO index.

I set up demo to make sure this is a UICollectionView problem, here is my code if you care to see it in action:

@implementation ViewController {
  UICollectionView *_collection;
  NSArray *_values;
}

- (instancetype)init
{
  self = [super init];
  if (self) {
    UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc] init];
    _collection = [[UICollectionView alloc] initWithFrame:CGRectZero collectionViewLayout:layout];
    _collection.dataSource = self;
    _values = @[@1, @2];
    [_collection registerClass:[UICollectionViewCell class]
      forCellWithReuseIdentifier:@"reuse"];
    [self.view addSubview:_collection];
  }
  return self;
}

- (void)viewDidLoad {
  [super viewDidLoad];
  _collection.frame = self.view.bounds;

  double delayInSeconds = 2.0;
  dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delayInSeconds * NSEC_PER_SEC));
  dispatch_after(popTime, dispatch_get_main_queue(), ^(void){
    [self triggerBatchUpdate];
  });
}

- (void)triggerBatchUpdate {
  [_collection performBatchUpdates:^{
    _values = @[@4, @5, @1];
    [_collection insertItemsAtIndexPaths:@[[self index:0]]];
    [_collection moveItemAtIndexPath:[self index:0] toIndexPath:[self index:2]];

    // Works with this line
//    [_collection moveItemAtIndexPath:[self index:1] toIndexPath:[self index:1]];

    // Fails with this line
    [_collection reloadItemsAtIndexPaths:@[[self index:1]]];
  } completion:nil];
}

- (NSIndexPath *)index:(NSUInteger)ind {
  return [NSIndexPath indexPathForRow:ind inSection:0];
}

- (NSInteger)collectionView:(UICollectionView *)collectionView
     numberOfItemsInSection:(NSInteger)section {
  return _values.count;
}

- (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView
                  cellForItemAtIndexPath:(NSIndexPath *)indexPath {
  UICollectionViewCell *cell = [_collection dequeueReusableCellWithReuseIdentifier:@"reuse"
                                                                      forIndexPath:indexPath];
  cell.backgroundColor = [UIColor grayColor];
  return cell;
}


@end

The fact that the code works if I use

[_collection moveItemAtIndexPath:[self index:1] toIndexPath:[self index:1]];

instead of

[_collection reloadItemsAtIndexPaths:@[[self index:1]]];

makes me suspicious. Those two operations should be equivalent.

Any idea what's going on here? Is this, as I believe, a UICollectionView bug?

EDIT: Another crash that turns out to be related to this:

attempt to perform a delete and a move from the same index path ( {length = 2, path = 0 - 5})

Dave
  • 534
  • 1
  • 5
  • 14
  • What if you perform the insert before the move? A move from 0 to 2 is invalid on a collection that only contains 2 items – Paulw11 Feb 09 '17 at 22:21
  • No that doesn't change anything, and it shouldn't. The order in which you do operations in a batchUpdate doesn't matter, they are performed in an apple-defined order (deletes first, then inserts, then moves). The move FROM index corresponds to its index in the original state, and the TO index is relative to the final state. – Dave Feb 09 '17 at 23:27
  • It looks like a bug. FYI, using the `[_collection moveItemAtIndexPath:[self index:1] toIndexPath:[self index:1]];` doesn't actually work as the second cell isn't reloaded. I used colors instead of numbers so you can see what is happening: https://gist.github.com/paulw11/025e6f3dbdfaf911a6dab95c6631cbe2 It only worked if I moved the reload to the completion block – Paulw11 Feb 10 '17 at 00:24
  • If I got rid of the move, left the insert and then tried to reload the item at index 2, I got an error that I was attempting to delete item 2 when there are only 2 items. It seems that a reload is converted into a delete and insert and this causes an issue where the indexes change during the batch. If I replace the reload with a discrete delete for index 1 and then an insert for index 1 it works – Paulw11 Feb 10 '17 at 00:30
  • Good catch! I was totally wrong with my move to itself assumption ( not sure where that came from, I wonder if it used to be true?) I think you're right, conversion from reload to delete and insert makes complete sense. There's been a very similar crash I haven't been able to reproduce with deletion and move, which reinforces your theory. Replacing reloads with a delete and insert seems to work fine :) You could write up an answer explaining all this and I'll accept it. If you'd rather not, that's fine and I'll do it for posterity. – Dave Feb 10 '17 at 01:15

1 Answers1

9

It does seem to be a bug, although while the documentation for performBatchUpdates:completion explains the context for indices in insert and delete operations, and mentions that reload operations are permitted, it doesn't detail the context for indices in reload operations.

After some experimentation, it seems that "under the covers" a reload is implemented as a delete and an insert. This seems to cause an issue where there is an overlap between some other operation and the reload index.

I did find, however, that replacing the reload with an explicit delete and insert seems to work, so you can use:

- (void)triggerBatchUpdate {
    [_collection performBatchUpdates:^{
        _values = @[[UIColor blueColor], [UIColor yellowColor], [UIColor redColor]];
        [_collection moveItemAtIndexPath:[self index:0] toIndexPath:[self index:2]];
        [_collection insertItemsAtIndexPaths:@[[self index:0]]];

        [_collection deleteItemsAtIndexPaths:@[[self index:1]]];
        [_collection insertItemsAtIndexPaths:@[[self index:1]]];
    } completion:nil];
}
Paulw11
  • 108,386
  • 14
  • 159
  • 186