gmoledina / GMGridView

A performant Grid-View for iOS (iPhone/iPad) that allows sorting of views with gestures (the user can move the items with his finger to sort them) and pinching/rotating/panning gestures allow the user to play with the view and toggle from the cellview to a fullsize display.
MIT License
2.3k stars 512 forks source link

[GMGridView insertObjectAtIndex] wrong assertion #48

Closed rantav closed 12 years ago

rantav commented 12 years ago

Maybe I'm not reading it right, but it seems that insertObjectAtIndex has a wrong assertion at the beginning.

- (void)insertObjectAtIndex:(NSInteger)index
{
    NSAssert((index >= 0 && index <= _numberTotalItems), @"Invalid index specified");

When calling insertObjectAtIndex then it's expected that since a new object is inserted, then index is > than _numberTotalItems (the number of total items should increase). That's at least how things work in UITableViewController, so I expected similar behavior here as well. Maybe my expectation is not in place though... what do you think?

mtrubnikov commented 12 years ago

I think it's ok. That means that inserted index may be in range from 0 (beginning of the gridview) up to _numberTotalItems - that index corresponds to a new item at the end of the gridview. Anything less than 0 or more than _numberTotalItems has no sense.

rantav commented 12 years ago

Mark, evidently when using nsfetchedresultscontroller then I get elements added in positions that could be greater than numTotalItems. Could be my mistake so I'll need to verify and update the issue. I've been out of office for a few days and I will still be out for a few more so I apologize that it's going to take a few days before I can report.

On Thursday, February 16, 2012, Mark Trubnikov wrote:

I think it's ok. That means that inserted index may be in range from 0 (beginning of the gridview) up to _numberTotalItems - that index corresponds to a new item at the end of the gridview. Anything less than 0 or more than _numberTotalItems has no sense.


Reply to this email directly or view it on GitHub: https://github.com/gmoledina/GMGridView/issues/48#issuecomment-4008302

/Ran http://tavory.com

rantav commented 12 years ago

I checked again and it seems that when using NSFetchedResultsController, the method controller:didChangeObject:atIndexPath:forChangeType:newIndexPath: with type NSFetchedResultsChangeInsert, does not report the insertion in an ascending order. It would indeed separately report the insertion of every new element in the results, but it would not do so in ascending order, so for example if you had 100 elements added the order might be 22, 44, 43, 98, 99, .... Strange, I know... maybe it's related to the way RestKit is loading objects in the background...

The documentation of NSFetchedResultsController doesn't clearly specify that it may or may not report insertion of new elements in specific order.

gmoledina commented 12 years ago

As @mtrubnikov said, I don't think there's a mistake there.

anujgakhar commented 12 years ago

So, is there a solution to how to use a FetchedResultsController successfully ? I too am getting stuck with this issue of the Assertion complaining about "Invalid Index specified" when using the method controller:didChangeObject:atIndexPath:forChangeType:newIndexPath:

rantav commented 12 years ago

@gmoledina perhaps you should consider reopening the bug. @anujgakhar my solution was to bypass the problem in a bit of an uglyish way... I'll paste my code

- (void)controller:(NSFetchedResultsController *)controller
   didChangeObject:(id)anObject
       atIndexPath:(NSIndexPath *)indexPath
     forChangeType:(NSFetchedResultsChangeType)type
      newIndexPath:(NSIndexPath *)newIndexPath {
  NSUInteger index = indexPath.row;
  NSUInteger newIndex = newIndexPath.row;
  switch(type) {
    case NSFetchedResultsChangeInsert:
      needsInserOfNewData = YES;
      // doesn't work as ecpected... see https://github.com/gmoledina/GMGridView/issues/48
      //[gmGridView insertObjectAtIndex:newIndex];
      [self hideEmpty];
      break;
    case NSFetchedResultsChangeDelete:
      [gmGridView removeObjectAtIndex:index];
      break;
    case NSFetchedResultsChangeUpdate:
      [gmGridView reloadObjectAtIndex:index];
      break;
    case NSFetchedResultsChangeMove:
      [gmGridView swapObjectAtIndex:index withObjectAtIndex:newIndex]; 
      break;
  }
}

- (void)controllerDidChangeContent:(NSFetchedResultsController *)controller {
  if (needsInserOfNewData) {
    [gmGridView reloadData];
  }
  needsInserOfNewData = NO;
}

In particular, pay attention to the use of needsInserOfNewData

anujgakhar commented 12 years ago

@rantav thanks for your sample code, this was exactly what I was thinking when I was thinking of a solution. To implement conditional reloadData. I hope this issue can be resolved at the source though.