refactor function so same code is written only once

Question

I have this code

@objc func didClickActionButton(sender: UITapGestureRecognizer) {

    if let contents = contentList {

      if contents[currentScreenIndex].customMeta["permission-location"] != nil {
        //request Permission
        ApiHelper.shared.initLocationHelperWithBeacon()
      }
      if contents[currentScreenIndex].customMeta["permission-notification"] != nil {
//        PushNotificationHelper.requestNotificationAuthorization { }
        PushNotificationHelper.requestNotificationAuthorization {
          DispatchQueue.main.async {
            if self.currentScreenIndex == contents.count - 1 {
                self.finishOnboarding()
              return
            }
           self.currentScreenIndex+=1
           self.showScreenAtPosition(position: self.currentScreenIndex)
          }
        }
        return
      }
      if currentScreenIndex == contents.count - 1 {
        finishOnboarding()
        return
      }
      currentScreenIndex+=1
      showScreenAtPosition(position: currentScreenIndex)
    }
  }

As you can see I have the same code in the DispatchQueue.main.async as I have at the bottom at the func. Is there anyway to refactor this so I don't write the same code twice? Is it even worth it to refactor this?

Answer 1

Refactor the repeated code into its own private function:

@objc func didClickActionButton(sender: UITapGestureRecognizer) {
    guard let contents = contentList else { return }

    if contents[currentScreenIndex].customMeta["permission-location"] != nil {
        //request Permission
        ApiHelper.shared.initLocationHelperWithBeacon()
    }

    if contents[currentScreenIndex].customMeta["permission-notification"] != nil {
        PushNotificationHelper.requestNotificationAuthorization {
            DispatchQueue.main.async {
                self.advanceOnboarding()
            }
        }
        return
    }

    advanceOnboarding()
}

private func advanceOnboarding() {
    if currentScreenIndex == contents.count - 1 {
        finishOnboarding()
        return
    }

    currentScreenIndex += 1
    showScreenAtPosition(position: currentScreenIndex)
}

I’d also suggest the guard for the early exit, as shown above.

Is it even worth it to refactor this?

Refactoring this code out into its own function not only avoids the unnecessary repetition (simplifying maintenance in the future), but it also makes this routine much easier to reason about. When first reading your code, it took a few seconds to figure out what you were trying to do, why that code was being repeated, etc. Giving that block of code a clear name makes it easier to grok what’s going on.