Kamil Harasimowicz
Kamil Harasimowicz

Reputation: 4984

Wrong IndexPath after insertion/deletion of cell from UITableView

Code

Essentials for the problem parts of VC:

// Part of VC where cell is setting
func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCell(for: indexPath) as Cell
    let cellVM = viewModel.cellVM(for: indexPath)

    cell.update(with: cellVM)
    cell.handleDidChangeSelectionState = { [weak self] selected in
        guard
            let `self` = self
        else { return }

        self.viewModel.updateSelectionState(selected: selected, at: indexPath)
    }

    return cell
}

// Part of code where cell can be deleted
func tableView(_ tableView: UITableView, editActionsForRowAt indexPath: IndexPath) -> [UITableViewRowAction]? {
    let deleteAction = UITableViewRowAction(style: .destructive, title: "delete".localized, handler: { [weak self] _, indexPath in
        guard let self = self else { return }

        self.viewModel.delete(at: indexPath)
        tableView.deleteRows(at: [indexPath], with: .left)
    })

    return [deleteAction]
}

Problem

When cell has been deleted and after that handleDidChangeSelectionState will be involved then indexPath passed into viewModel.updateSelectionState will be wrong (will be equal to value before cell deletion).

I think I know why

  1. IndexPath is a struct so handleDidChangeSelectionState keeps copy of current value (not instance). Any update of original value will not update captured copy.
  2. tableView.deleteRows will not reload datasource of tableview so cellForRowAt will not recall. It means handleDidChangeSelectionState will not capture updated copy.

My to approach to fix that problem

* 1st

Ask about indexPath value inside handleDidChangeSelectionState:

cell.handleDidChangeSelectionState = { [weak self, weak cell] selected in
    guard
        let `self` = self,
        let cell = cell,
        // now I have a correct value
        let indexPath = tableView.indexPath(for: cell)
    else { return }

    self.viewModel.updateSelectionState(selected: selected, at: indexPath)
}

* 2nd

After every deletion perform reloadData():

    let deleteAction = UITableViewRowAction(style: .destructive, title: "delete".localized, handler: { [weak self] _, indexPath in
        guard let self = self else { return }

        self.viewModel.delete(at: indexPath)
        tableView.deleteRows(at: [indexPath], with: .left)

        // force to recall `cellForRowAt` then `handleDidChangeSelectionState` will capture correct value
        tableView.reloadData()
    })

Question

Which approach is better?

I want to:

Maybe there is a better 3rd approach.

Thanks for any advice.

Upvotes: 0

Views: 514

Answers (2)

AtulParmar
AtulParmar

Reputation: 4570

Don't use reload specific row, because the index has changed when you delete row, and it will remove animation after deleting row that's why you need to reload tableview.reloadData() it is a better option in your case.

let deleteAction = UITableViewRowAction(style: .destructive, title: "delete".localized, handler: { [weak self] _, indexPath in
        guard let self = self else { return }

        self.viewModel.delete(at: indexPath)
        tableView.deleteRows(at: [indexPath], with: .left)

        // force to recall `cellForRowAt` then `handleDidChangeSelectionState` will capture correct value
        tableView.reloadData()
    })

Upvotes: 0

vadian
vadian

Reputation: 285059

Only the 1st approach fulfills the first condition to keep smooth animations. Calling reloadData right after deleteRows breaks the animation.

And calling indexPath(for: cell) is certainly less expensive than reloading the entire table view.

Upvotes: 2

Related Questions