CODEWITHSUNDEEP
iosswiftuitableviewtableviewrace-condition
Eric Conner
Eric Conner

Reputation: 10752

Is it unsafe to call reloadData() after getting an indexPath but before removing a cell at that indexPath?

I'm trying to track down a difficult crash in an app.

I have some code which effectively does this:

if let indexPath = self.tableView.indexPath(for: myTableViewCell) {
    // .. update some state to show a different view in the cell ..
    self.tableView.reloadData()

    // show nice fade out of the cell
    self.friendRequests.remove(at: indexPath.row)
    self.tableView.deleteRows(at: [indexPath], with: .fade)
}

The concern is that calling reloadData() somehow makes the indexPath I just retrieved invalid so the app crashes when it tries to delete the cell at that indexPath. Is that possible?

Edit:

The user interaction is this:

  1. User taps a button [Add Friend] inside of table view cell <-- indexPath retrieved here
  2. Change the button to [Added] to show the tap was received. <-- reloadData called here
  3. Fade the cell out after a short delay (0.5s). <-- delete called here with indexPath from #1

I can change my code to not call reloadData and instead just update the view of the cell. Is that advisable? What could happen if I don't do that?

Upvotes: 0

Views: 231

Answers (2)

Rob
Rob

Reputation: 437552

Personally, I'd just reload the button in question with reloadRows(at:with:), rather than the whole table view. Not only is this more efficient, but it will avoid jarring scrolling of the list if you're not already scrolled to the top of the list.

I'd then defer the deleteRows(at:with:) animation by some small fraction of time. I personally think 0.5 seconds is too long because a user may proceed to tap on another row and they can easily get the a row other than what they intended if they're unlucky enough to tap during the wrong time during the animation. You want the delay just long enough so they get positive confirmation on what they tapped on, but not long enough to yield a confusing UX.

Anyway, you end up with something like:

func didTapAddButton(in cell: FriendCell) {
    guard let indexPath = tableView.indexPath(for: cell), friendsToAdd[indexPath.row].state == .notAdded else {
        return
    }

    // change the button

    friendsToAdd[indexPath.row].state = .added
    tableView.reloadRows(at: [indexPath], with: .none)

    // save reference to friend that you added

    let addedFriend = friendsToAdd[indexPath.row]

    // later, animate the removal of that row

    DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
        if let row = self.friendsToAdd.index(where: { $0 === addedFriend }) {
            self.friendsToAdd.remove(at: row)
            self.tableView.deleteRows(at: [IndexPath(row: row, section: 0)], with: .fade)
        }
    }
}

(Note, I used === because I was using a reference type. I'd use == with a value type that conforms to Equatable if dealing with value types. But those are implementation details not relevant to your larger question.)

That yields:

enter image description here

Upvotes: 1

Zapko
Zapko

Reputation: 2461

Yes, probably what's happening is the table view is invalidating stored index path.

To test whether or not it is the issue try to change data that is represented in the table right before reloadData() is called.

If it is a problem, then you'll need to use an identifier of an object represented by the table cell instead of index path. Modified code will look like this:

func objectIdentifer(at: IndexPath) -> Identifier? {
   ...
}

func indexPathForObject(_ identifier: Identifier) -> IndexPath? {
   ...
}

if 
    let path = self.tableView.indexPath(for: myTableViewCell) 
    let identifier = objectIdentifier(at: path) {

    ...

    self.tableView.reloadData()

    ...

    if let newPath = indexPathForObject(identifier) {
        self.friendRequests.removeObject(identifier)
        self.tableView.deleteRows(at: [newPath], with: .fade)
    }
}

Upvotes: 0

Related Questions