Deleting item in ForEach loop causes fatal error: Index out of range

Question

I have a ScrollView which displays a list of rows using a ForEach loop from an array. When I delete an item from the array, I get the error: Index out of range.

ScrollView {
    ForEach(viewModel.tasks.indices, id: \.self){ index in
        TaskRow(
            task: self.$viewModel.tasks[index],
            deleteAction: {
                self.viewModel.deleteTask(task: self.viewModel.tasks[index])
            }
        )
    }
}

This error only started occurring when I switched to passing the index from the ForEach loop instead of the 'task' itself. I had to do this so I could use a @Binding var task: Task in the subview: "TaskRow"

The 'delete action' is triggered by a button in the subview.

viewModel.deleteTask works as follows (using a dataManager):

final class StackDetailViewModel: ObservableObject {
    
    @Published var tasks = [Task]()
    
    var dataManager: DataManagerProtocol
    
    init(dataManager: DataManagerProtocol = DataManager.shared){
        self.dataManager = dataManager
        fetchTasks()
    }
}
extension StackDetailViewModel {

    func fetchTasks() {
        tasks = dataManager.fetchTasks()
    }

    func deleteTask(task: Task) {
        dataManager.deleteTask(task: task)
        fetchTasks()
    }

}

Where the dataManager does this:


Class DataManager {

...

    private var tasks = [Task]()

...
 
    func fetchTasks() -> [Task] {
        tasks
    }

    func deleteTask(task: Task) {
        if let index = tasks.firstIndex(where: { $0.id == task.id }) {
            tasks.remove(at: index)
        }
    }

}

I use protocols in my app but I've removed them here for simplicity. Any help would be greatly appreciated.

Based on what you showed, it should work. I suspect the bug is somewhere between DataManager and your ViewModel. — New Dev
in general, you do not want to delete from an array while stepping through it — Rick
@NewDev From the tests i've done. The error only occurs when I change from using ForEach(viewModel.tasks){ task in to what I have shown above. Keeping the dataManager and ViewModel the same. — santi.gs

pd95 pd95 · Accepted Answer · 2021-03-14T23:09:08

It's not good practice to use the array indices directly in ForEach as identifiers, because indices are positional and do not identify the item which they represent. This can cause redraw issues and crashes in SwiftUI.
E.g. when you use "swipe to delete" in a regular List, SwiftUI knows that the item at the given position has been deleted and will probably not ask again for the full list of IDs (which are indices in this incorrect case) to redraw the content. SwiftUI will simply provide the list of remaining IDs for the next redraw, which is leading to an out-of-bounds crash.

I don't mean that this is the exact case here as I was not able to reproduce your crash.

As you write yourself the problem appears when you modify the ForEach to use indices instead of your tasks.

Instead of

ForEach(viewModel.tasks.indices, id: \.self){ index in

Use

ForEach(Array(viewModel.tasks.enumerated()), id: \.1.id) { (index, task) in

This gives you access to the index for the Binding to the task needed for TaskRow and the task associated. Further by specifying the tasks ID as identifier for each row, SwiftUI will not rely on the positional indices anymore.

But beware: this solution might not perform as well for very large arrays of tasks, because the EnumeratedSequence (returned by .enumerated() on your tasks Array) is flattened into a new Array.

So if you are working with large lists of tasks, I would recommend your initial version: ForEach(viewModel.tasks) and eventually pay the performance penalty in the case of deleteAction only.

Deleting item in ForEach loop causes fatal error: Index out of range

1 Answers