MasterDetail Swift CoreData Template Suggestion

Originator:indiekiduk
Number:rdar://FB7730495 Date Originated:Jun 9, 2020 at 1:11 PM
Status:Open Resolved:No
Product:Xcode Product Version:11.5
Classification:Suggestion Reproducible:Always
 
Please provide a descriptive title for your feedback:
MasterDetail Swift CoreData Template Suggestion
Which area are you seeing an issue with?
Xcode
What type of feedback are you reporting?
Suggestion
Details
What version of Xcode are you using?
Xcode 11.5
Description
Please describe the issue:
MasterDetail Swift CoreData Template Suggestiong

This project demonstrates an issue with the Swift Master Detail template as of Xcode 11.5 and I'd like to suggest an improvement to solve it.

The Master's Fetch Controller didChange assumes that an updated object's cell will always be on screen and will crash if it isn't. This is a regression compared to the ObjC version of the template that doesn't crash if an updated cell is on screen because of how it handles the cell being nil.

This code began with the MasterDetail Core Data Swift teplate and has a few things added to demonstrate the crash.

1. A tool bar button is added to the Master view controller.
2. An IBAction is added to update the first object with a new timestamp which causes the cell to be updated.

How to use.

1. Launch in the simulator, e.g. iPhone 8 Plus.
2. Tap the plus button to add a cell.
3. Tap the Update tool bar button and notice the first row updates.
4. Now add more than a full screen height of rows.
5. Scroll down so the first row is now off-screen.
6. Tap the Update toolbar button.

What you'd expect to happen.
The app shouldn't crash.

What happens:
The app crashes.

I've included a proposal for a change to the Master detail template to fix this problem, by checking if the cell is not null (and thus is on screen) using an if let. I've also improved it to use the fallthrough feature so the configure method doesn't need to be called twice with the same params.


/// Default Method From Master Detail Template (see below for proposed fixed method):
    
    func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChange anObject: Any, at indexPath: IndexPath?, for type: NSFetchedResultsChangeType, newIndexPath: IndexPath?) {
        switch type {
            case .insert:
                tableView.insertRows(at: [newIndexPath!], with: .fade)
            case .delete:
                tableView.deleteRows(at: [indexPath!], with: .fade)
            case .update:
                configureCell(tableView.cellForRow(at: indexPath!)!, withEvent: anObject as! Event)
            case .move:
                configureCell(tableView.cellForRow(at: indexPath!)!, withEvent: anObject as! Event)
                tableView.moveRow(at: indexPath!, to: newIndexPath!)
            default:
                return
        }
    }
    
/// Proposed fixed method:

    func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChange anObject: Any, at indexPath: IndexPath?, for type: NSFetchedResultsChangeType, newIndexPath: IndexPath?) {
        switch type {
            case .insert:
                tableView.insertRows(at: [newIndexPath!], with: .fade)
            case .delete:
                tableView.deleteRows(at: [indexPath!], with: .fade)
            case .move:
                tableView.moveRow(at: indexPath!, to: newIndexPath!)
                fallthrough
            case .update:
                if let cell = tableView.cellForRow(at: indexPath!) {
                    configureCell(cell, withEvent: anObject as! Event)
                }
            default:
                return
        }
    }

Comments


Please note: Reports posted here will not necessarily be seen by Apple. All problems should be submitted at bugreport.apple.com before they are posted here. Please only post information for Radars that you have filed yourself, and please do not include Apple confidential information in your posts. Thank you!