UITableView allowing some kinds of requests to be deleted with swipe gestures

Question

I have a tableview which shows a list of objects called Requests. It has 3 segments. Namely Accepted, Received and Sent. And the objects for each segment are in 3 arrays.

I want to enable deleting rows (in turn the objects) only for Accepted and Sent segments.

This is my current implementation.

func tableView(_ tableView: UITableView, trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath) -> UISwipeActionsConfiguration? {

    var deleteAction: UIContextualAction?

    switch currentShowingStatus {
    case .accepted:
        deleteAction = UIContextualAction(style: .destructive, title: "Delete") { action, view, completionHandler in
            let request = self.acceptedRequests[indexPath.row]
            completionHandler(true)
        }
    case .sent:
        deleteAction = UIContextualAction(style: .destructive, title: "Delete") { action, view, completionHandler in
            let request = self.sentRequests[indexPath.row]
            completionHandler(true)
        }
    default:
        break
    }


    if let deleteAction = deleteAction {
        let configuration = UISwipeActionsConfiguration(actions: [deleteAction])
        configuration.performsFirstActionWithFullSwipe = false
        return configuration
    } else {
        return UISwipeActionsConfiguration(actions: [])
    }
}

As you can see there is quite a bit of duplicating code. And I can't extract away the UIContextualAction declaration part because of it's completion handler. Selecting the object to be deleted happens in there.

I also can't define UISwipeActionsConfiguration as a local variable and reduce its duplicating due to that class must be initialized with UIContextualAction instances.

Note: You have to pass an empty actions array to UISwipeActionsConfiguration in order to not show the swipe actions in cell. Returning nil doesn't do it.

So all this has produced an ugly piece of code. I'm wondering if there's a better way to refactor this.

---

Alternative Approach

Instead of checking for segment first and then adding multiple delete actions, I put the checking for segment part inside one completion handler like this.

func tableView(_ tableView: UITableView, trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath) -> UISwipeActionsConfiguration? {
    var actions = [UIContextualAction]()

    let deleteAction = UIContextualAction(style: .destructive, title: "Delete") { action, view, completionHandler in
        switch self.currentShowingStatus {
        case .accepted:
            let request = self.acceptedRequests[indexPath.row]
            self.deleteClientRequest(request)
        case .sent:
            let request = self.sentRequests[indexPath.row]
            self.deleteClientRequest(request)
        default:
            break
        }
        completionHandler(true)
    }

    switch currentShowingStatus {
    case .accepted, .sent:
        actions.append(deleteAction)
    case .received:
        actions.removeAll()
    }

    let configuration = UISwipeActionsConfiguration(actions: actions)
    configuration.performsFirstActionWithFullSwipe = false
    return configuration
}

The downside is I still have to check for segment again in order to show the swipe action for just the Accepted and Sent segments.

Answer 1

In your first implementation you can make deleteAction a constant if you initialize it in all cases of the switch-statement:

    let deleteAction: UIContextualAction?
    switch currentShowingStatus {
    case .accepted:
        deleteAction = UIContextualAction(...)
    case .sent:
        deleteAction = UIContextualAction(...)
    case .received:
        deleteAction = nil
    }

That makes it clear that the variable is initialized exactly once before used. I would also replace default: by the explicit case received:. That makes it obvious to reader in which case no swipe action is configured (without looking up the enum definition) and forces you to update the code if more cases are added.

The double definition of UISwipeActionsConfiguration could be avoided by initializing the array of actions instead:

    let actions: [UIContextualAction]
    switch currentShowingStatus {
    case .accepted:
        actions = [UIContextualAction(...)]
    case .sent:
        actions = [UIContextualAction(...)]
    default:
        actions = []
    }

    let configuration = UISwipeActionsConfiguration(actions: actions)
    configuration.performsFirstActionWithFullSwipe = false
    return configuration

It remains the code duplication for UIContextualAction though.

In your alternative approach you append or remove elements from the actions array. But that array is initially empty. Similarly as above, you can simplify it to

    let deleteAction = ...

    let actions: [UIContextualAction]
    switch currentShowingStatus {
    case .accepted, .sent:
        actions = [deleteAction]
    case .received:
        actions = []
    }

But – as you noticed – you have to evaluate self.currentShowingStatus inside the closure again. I would avoid that for an additional reason: You are relying on the fact that the status has not changed when the closure is executed.

The code duplication for UIContextualAction can be avoided if you initialize the request first:

   let request: Request?
   switch currentShowingStatus {
   case .accepted:
       request = self.acceptedRequests[indexPath.row]
   case .sent:
       request = self.sentRequests[indexPath.row]
   case .received:
       request = nil
   }

   if let request = request {
       let deleteAction = UIContextualAction(style: .destructive, title: "Delete") {
           _, _, completionHandler in
           self.deleteClientRequest(request)
           completionHandler(true)
       }
       let configuration = UISwipeActionsConfiguration(actions: [deleteAction])
       configuration.performsFirstActionWithFullSwipe = false
       return configuration
   } else {
       return UISwipeActionsConfiguration(actions: [])
   }

Note also that unused closure parameters can be replaced by the wildcard parameter _.

I would probably leave it like that, but of course you can combine the techniques:

    let request: Request?
    switch currentShowingStatus {
    case .accepted:
        request = self.acceptedRequests[indexPath.row]
    case .sent:
        request = self.sentRequests[indexPath.row]
    case .received:
        request = nil
    }

    let actions: [UIContextualAction]
    if let request = request {
        actions = [UIContextualAction(style: .destructive, title: "Delete") {
            _, _, completionHandler in
            self.deleteClientRequest(request)
            completionHandler(true)
            }]
    } else {
        actions = []
    }

    let configuration = UISwipeActionsConfiguration(actions: actions)
    configuration.performsFirstActionWithFullSwipe = false
    return configuration