Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on update in CollectionViewDataSource #20

Open
georgeharker opened this issue May 23, 2016 · 1 comment
Open

Crash on update in CollectionViewDataSource #20

georgeharker opened this issue May 23, 2016 · 1 comment

Comments

@georgeharker
Copy link

I'm experimenting with the code in a slightly different context (so there may be no bug in the example app), but I believe there's a lurking bug in the implementation of

processUpdates(updates: [DataProviderUpdate<Data.Object>]?)

if an update is received for an object (say via the FetchedResultsDataProvider invoking it's delegate - DataProviderDelegate.dataProviderDidUpdate(updates: [DataProviderUpdate<Object>]?)) for an object which is known and in the model, but has become off-screen, then inside the batch update's .Update(let indexPath, let object): clause, then the line

guard let cell = self.collectionView.cellForItemAtIndexPath(indexPath) as? Cell else { fatalError("wrong cell type") }

will fail because self.collectionView.cellForItemAtIndexPath(indexPath) can return nil for cells which are out of range, or simply because they're offscreen. If there's no cell, it's likely the right approach is to check for a valid cell before proceeding and retain the type check on the cell type for safety (i.e.. split out the nil check from the type check)... something like

                case .Update(let indexPath, let object):
                    // Note: cell can be offscreen and therefore nil
                    if let c = self.collectionView.cellForItemAtIndexPath(indexPath) {
                        guard let cell = c as? Cell else { fatalError("wrong cell type") }
                        cell.configureForObject(object)
                    }
@need2edit
Copy link

@georgeharker stubbled across this post, we too had to implement a check for nil cells. Our context was utilizing lots of different custom cell types. Great catch and +1 on this change to implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants