Better way to provide data to UIKit component from Combine pipeline

Question

Today I'm researching how to provide data to UIKit component when using Combine pipeline.

I could see tons of tutorials about how to use SwiftUI + Combine from internet, but not found anyone helpful stuff about UIKit + Combine.

Well, I list one approach here, I put some data works in WebService and mean time receive the data in ViewController with Combine pipeline and provide them as the data source of tableView.

But I could see the fetchData() is mixed in ViewController now, it should be wonderful if I could put it into WebService and isolate the data pipeline operation with ViewController.

So what is the better way to provide the data to UIKit component when using Combine pipeline like dataTaskPublisher? Is my current approach the common way to do it?

Approach 1

WebService.swift

import Combine
import UIKit

enum HTTPError: LocalizedError {
    case statusCode
    case post
}

enum FailureReason: Error {
    case sessionFailed(error: HTTPError)
    case decodingFailed
    case other(Error)
}

struct Response: Codable {
    let statusMessage: String?
    let success: Bool?
    let statusCode: Int?
}

class WebService {
    private var requests = Set<AnyCancellable>()
    
    private var decoder: JSONDecoder = {
        let decoder = JSONDecoder()
        decoder.keyDecodingStrategy = .convertFromSnakeCase
        return decoder
    }()
    
    private var session: URLSession = {
        let config = URLSessionConfiguration.default
        config.allowsExpensiveNetworkAccess = false
        config.allowsConstrainedNetworkAccess = false
        config.waitsForConnectivity = true
        config.requestCachePolicy = .reloadIgnoringLocalCacheData
        return URLSession(configuration: config)
    }()
    
    func createPublisher<T: Codable>(for url: URL) -> AnyPublisher<T, FailureReason> {
        return session.dataTaskPublisher(for: url)
            .tryMap { output in
                guard let response = output.response as? HTTPURLResponse, response.statusCode == 200 else {
                    throw HTTPError.statusCode
                }
                return output.data
            }
            .decode(type: T.self, decoder: decoder)
            .mapError { error in
                switch error {
                case is Swift.DecodingError:
                    return .decodingFailed
                case let httpError as HTTPError:
                    return .sessionFailed(error: httpError)
                default:
                    return .other(error)
                }
            }
            .eraseToAnyPublisher()
    }
    
    func getPetitionsPublisher(for url: URL) -> AnyPublisher<Petitions, FailureReason> {
        createPublisher(for: url)
    }
}

ViewController

import Combine
import UIKit

class ViewController: UIViewController, UITableViewDelegate, UITableViewDataSource {
    var tableView = UITableView()
    var petitions = [PetitionViewModel]()
    let webService = WebService()
    private var cancellableSet = Set<AnyCancellable>()
    

    override func viewDidLoad() {
        super.viewDidLoad()
        
        view.backgroundColor = .systemBackground
        title = "Petitions"
        navigationItem.rightBarButtonItem = UIBarButtonItem(title: "Credits", style: .plain, target: self, action: #selector(rightBarButtonTapped))
        
        tableView.delegate = self
        tableView.dataSource = self
//        tableView.register(UITableViewCell.self, forCellReuseIdentifier: "cell")
        tableView.translatesAutoresizingMaskIntoConstraints = false
        view.addSubview(tableView)
        
        let g = view.safeAreaLayoutGuide
        NSLayoutConstraint.activate([
            tableView.leadingAnchor.constraint(equalTo: g.leadingAnchor),
            tableView.trailingAnchor.constraint(equalTo: g.trailingAnchor),
            tableView.topAnchor.constraint(equalTo: g.topAnchor),
            tableView.bottomAnchor.constraint(equalTo: g.bottomAnchor)
        ])
        
    }
    
    override func viewWillAppear(_ animated: Bool) {
        let url = WhiteHouseClient.petitions.url
        print("url: \(url)")
        fetchData(for: url)
        print("viewWillAppear")
    }
    
    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return petitions.count
    }
    
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "cell") ?? UITableViewCell(style: .subtitle, reuseIdentifier: "cell")
        cell.accessoryType = .disclosureIndicator
        
        let petition = petitions[indexPath.row]
        cell.textLabel?.text = petition.name
        cell.detailTextLabel?.text = petition.body
        
        return cell
    }
    
    @objc func rightBarButtonTapped() {
        let ac = UIAlertController(title: "Credits", message: "the data comes from the We The People API of the Whitehouse.", preferredStyle: .alert)
        ac.addAction(UIAlertAction(title: "OK", style: .default))
        
        present(ac, animated: true)
    }
    
    func fetchData(for url: URL) {
        
        webService.getPetitionsPublisher(for: url)
            .receive(on: DispatchQueue.main)
            .sink(receiveCompletion: { status in
                switch status {
                case .finished:
                    break
                case .failure(let error):
                    print(error)
                    break
                }
            }) { [unowned self] petitions in
                self.petitions = petitions.results.map(PetitionViewModel.init)
                DispatchQueue.main.async {
                    self.tableView.reloadData()
                }
            }.store(in: &self.cancellableSet)
        
    }
}

Answer 1

This looks fine. A bunch of minor observations:

• WebService.swift should import Foundation, not UIKit. A network layer should not be imposing any platform-specific constraints.

• T in createPublisher only needs to conform to Decodable. Do not impose unnecessary conformance requirements.

• The getPetitionsPublisher doesn't belong in WebService. We would want to avoid entangling the WebService with any particular model type, such as Petitions. I would pull this out and move it to its own extension. Or, better, since this isn't doing anything other than specializing the generic, just specify the data type in fetchData, and getPetitionsPublisher is rendered completely unnecessary and can be deleted. But, avoid coupling a network layer with specific model types, if you can.

• I would also rename FailureReason with something that indicates the domain and type of the failure, e.g. WebServiceError.

• I would also rename the generic createPublisher to publisher or webServicePublisher, to follow standard naming conventions. Apple does not use a verb in their publisher methods, and I would suggest following that convention.

• Given that WebService is creating a URLSession, make sure to invalidate the session when you are done with it. Otherwise, you will leak.

  class WebService {
      private let decoder: JSONDecoder = ...
      private let session: URLSession = ...
  
      deinit {
          session.finishTasksAndInvalidate()
      }
      ...
  }
  

  Note, you are not using requests, so that property can be removed.

• You might also consider making WebService a singleton, rather than repeating reinstantiating it.

• You might define a protocol for the WebService. That way, when writing unit tests, you can stub the web service.

  enum WebServiceError: Error {
      case sessionFailed(error: HTTPError)
      case decodingFailed
      case other(Error)
  }
  
  protocol WebServiceInterface {
      func publisher<T: Decodable>(for url: URL) -> AnyPublisher<T, WebServiceError>
  }
  
  class WebService {
      private let decoder: JSONDecoder = ...
      private let session: URLSession = ...
  
      deinit {
          session.finishTasksAndInvalidate()
      }
  }
  
  // MARK: - WebServiceInterface
  
  extension WebService: WebServiceInterface {
      func publisher<T: Decodable>(for url: URL) -> AnyPublisher<T, WebServiceError> {
         session.dataTaskPublisher(for: url)
              .tryMap { output in
                  guard
                      let response = output.response as? HTTPURLResponse,
                      200 ..< 300 ~= response.statusCode
                  else {
                      throw HTTPError.statusCode
                  }
                  return output.data
              }
              .decode(type: T.self, decoder: decoder)
              .mapError { error in
                  switch error {
                  case is Swift.DecodingError:     return .decodingFailed
                  case let httpError as HTTPError: return .sessionFailed(error: httpError)
                  default:                         return .other(error)
                  }
              }
              .eraseToAnyPublisher()
      }
  }
  

  And then the class that is using the web service can specify the interface:

  private var webService: WebServiceInterface = WebService()
  

• Note, I would suggest considering all values in the range of 200..<300 as success. Your current web service might only return 200, but in reality, all 2xx codes are success.

• In fetchData,

  • Your second case does not need a break;
  • Since you are really only trying to handle a single case, you should probably use if case let syntax;
  • When calling sink, I might suggest the sink { completion in ... } receiveValue { value in ... } syntax as enabled by SE-0279 Multiple Trailing Closures. It avoids that messy sink { status in ... } } ) { ... } syntax.
  • One should not use unowned with asynchronous methods because, while it avoids keeping a strong reference, if the view controller was dismissed by the time the closure was called, your code would crash. Use weak if you want to avoid strong references with asynchronous closures. Only use unowned in cases where you know that the captured object can never be deallocated by the time the closure is called.
  • You are dispatching the reload of the table view to the main queue, but not the update to the model. If your closure was being called on a background thread, then both the model update and the reload of the table should be inside the dispatch to the main queue. Or, because you already have a .receive(on: DispatchQueue.main), the dispatch to the main queue is now redundant.

  Thus:

  func fetchData(for url: URL) {
      webService.publisher(for: url)
          .receive(on: DispatchQueue.main)
          .sink { completion in
              if case .failure(let error) = completion {
                  print(error)
              }
          } receiveValue: { [weak self] (petitions: Petitions) in
              self?.petitions = petitions.results.map(PetitionViewModel.init)
              self?.tableView.reloadData()
          }.store(in: &cancellables)
  }
  

• I'm not sure why you are mapping the petitions.results to PetitionViewModel.init. You have not shared your model declarations, so we cannot comment on that, but generally we would have the decoder do that conversion for us.

• In viewWillAppear, make sure to call super.

• I might suggest breaking ViewController into some extensions. E.g., you might move UITableViewDataSource and UITableViewDelegate conformance into extensions, and move the private utility methods into their own private extension. It makes it easier to grok which methods are being used for what purpose and which are private. Also, as you working on your view controller, you can collapse sections unrelated to what you that. I would also break the viewDidLoad into logical functions, so a reader does not get lost in the details.

  For example:

  class ViewController: UIViewController {
      private var tableView = UITableView()
      private var petitions: [PetitionViewModel] = []
      private var webService: WebServiceInterface = WebService()
      private var cancellables: = Set<AnyCancellable>()
  
      override func viewDidLoad() {
          super.viewDidLoad()
  
          configureViews()
      }
  
      override func viewWillAppear(_ animated: Bool) {
          super.viewWillAppear(animated)
  
          fetchData(for: WhiteHouseClient.petitions.url)
      }
  }
  
  // MARK: - Private utility methods
  
  private extension ViewController {
      func configureViews() {
          view.backgroundColor = .systemBackground
          title = "Petitions"
          navigationItem.rightBarButtonItem = UIBarButtonItem(title: "Credits", style: .plain, target: self, action: #selector(didTapRightBarButton(_:)))
  
          configureTableView()
      }
  
      func configureTableView() {
          tableView.delegate = self
          tableView.dataSource = self
          tableView.register(SubtitleCell.self, forCellReuseIdentifier: "PetitionCell")
          tableView.translatesAutoresizingMaskIntoConstraints = false
          view.addSubview(tableView)
  
          let guide = view.safeAreaLayoutGuide
          NSLayoutConstraint.activate([
              tableView.leadingAnchor.constraint(equalTo: guide.leadingAnchor),
              tableView.trailingAnchor.constraint(equalTo: guide.trailingAnchor),
              tableView.topAnchor.constraint(equalTo: guide.topAnchor),
              tableView.bottomAnchor.constraint(equalTo: guide.bottomAnchor)
          ])
      }
  
      @objc func didTapRightBarButton(_ sender: UIBarButtonItem) { ... }
  
      func fetchData(for url: URL) { ... }
  }
  
  // MARK: - UITableViewDataSource
  
  extension ViewController: UITableViewDataSource {
      func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { ...}
  
      func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { ... }
  }
  
  // MARK: - UITableViewDelegate
  
  extension ViewController: UITableViewDelegate { }
  

• You have commented out the registration of the cell identifier. That means that you have lost any cell reuse capabilities. I would define a UITableViewCell subclass that is configured the way you would like it, and reuse that.

• You might consider giving rightBarButtonTapped a sender parameter, which is a UIBarButtonItem. It makes it easier, at a glance, to understand what it is doing. I might call it didTapRightBarButton to follow standard naming conventions, too.

• You might consider abstracting the fetchData routine out of the view controller, especially as the view controller grows in complexity.

  View controllers are best limited to view-related code. Business logic, interaction with network API, interaction with data stores, etc., are best contained within a UI-independent object that can be more easily unit tested.

Despite the plethora of observations above, I think you had a good start. But hopefully the above gives you some ideas.

Answer 2

Instead of let webService = WebService(), suppose your viewController took a function that creates/returns the publisher when needed: var makePublisher: () -> AnyPublisher<[PetitionViewModel], FailureReason>, so the vc can call it lazily, perhaps in viewWillAppear. Now the vc doesn't know anything about the webService, and it could easily be wired up to a test publisher with static responses.

But be careful about viewWillAppear: it's possible that it can be called more than once, (and also that you never get viewDidAppear, or you're not onscreen when the fetch returns) and consider renaming your fetchData(for: url) method - it doesn't directly fetch data, it creates a new subscription and adds it to the cancellableSet. If you mean to tear down and recreate the pipeline on each viewWillAppear, consider storing the cancellable in its own property, so you can have at most 1 of them.