Fetching data closure completion

Question

I started learning MVVM and API data fetching, so I made this code:

Function loadDirector in which directorName value is modified with fetched data:

func loadDirector(id: Int, completed: @escaping () -> ()) {
    homeRepository.getDirector(id: id){ (creditReponse) -> (Void) in
        self.directorName = ""
        creditReponse?.crew.forEach({ singleValue in
            if singleValue.knownForDepartment == .directing {
                self.directorName = singleValue.name
                
            }
        })
        completed()
    }
}

Function getDirector which is used for data fetch from Internet:

func getDirector(id:Int, _ completed: @escaping (CreditsResponse?) -> (Void)) {
    movieServiceAPI.fetchData(from: NetworkData.directorUrl(id: id).value, by: completed)
}

I am using loadDirector function in didSelectRowAt tableview function from my main VC:

func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
    
    let movie = homeViewModel.movieList[indexPath.row]
    homeViewModel.loadDirector(id: movie.id, completed: {

        self.changeVC(movie: movie, director: (self.homeViewModel.directorName), groups: self.homeViewModel.setupGenres(groups: movie.genreIds), movieIndex: indexPath.row)
        
    })
}

Code is working fine, but my question is how I can refactor code to switch completion handler from tableview function to viewmodel file so I can make call like this:

func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {

let movie = homeViewModel.movieList[indexPath.row]
homeViewModel.loadDirector(id: movie.id)

self.changeVC(movie: movie, director: (self.homeViewModel.directorName), groups: 
   self.homeViewModel.setupGenres(groups: movie.genreIds), movieIndex: indexPath.row)

})
}

Answer 1

The basic premise behind MVVM is that you have a ViewModel that tells the views in the view controller how they should look. The only properties that your view controller should have are the views it contains and a single view model object. No other properties should exist in it. Your view controller should be as dumb as possible...

For example, your tableView(_:didSelectRowAt:) method should look like this:

func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
    viewModel.selected(indexPath: indexPath)
}

That's it.

In your view controller's viewDidLoad, you would have something like this:

viewModel.subscribe { view in
    // here you would update all your views' properties based on the values in the `view` struct.
    // call `tableView.reloadData()` if a Bool in your view indicates that you should.
    // make sure you use weak or unowned self inside this closure.
}

To populate your table view, you would have something like:

func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
    viewModel.numberOfRowsInSection(section)
}

and

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCell(withIdentifier: "cellTypeIdentifier", for: indexPath)
    let cellView = viewModel.cellViewForRowAt(indexPath)
    cell.configure(with: cellView)
    return cell
}

Only reference your viewModel once in each method and don't store other properties in your view controller (other than the views) and you will be much closer to properly implementing the pattern.

Answer 2

What I am missing in your code, is a clear definition of the what the view (which is also your View Controller) should render. That value basically constitutes the "binding".

Let's give it a name and make a struct for this, struct ViewState {...}.

Introducing ViewState

From the perspective of the View this is the "single source of truth". A view does not know or need anything else. The view state completely determines how it renders itself.

The view state is const, means, the view will not mutate it. However, the view (ViewController) will maintain a copy of it in a property and updates the copy whenever it finished rendering the new state:

private var viewState: ViewState = .init()

It's obvious, that the view state gets generated by the view model and the view observes it. When the value changes, the view renders the new view state, possibly performing nice animations from the difference of the new and the old view state.

Actions

What's also required is a means to signal "events" (aka "actions") from the view (a user clicks the "Submit" button, etc.) to the view model.

That means, your View Model should have corresponding functions which perform such events respectively actions. Those can also have parameters.

Crafting a ViewState

When I look into your code, your view state is literally "anaemic" ;) I mean, you should enrich it with more information, so the view can render appropriately.

As your function which fetches data is asynchronous, you have an initial situation in your view which shows "no content". The view must know how to draw absent data - and the view model should tell the view what to render in this case.

Your users also may benefit from the fact, that they get informed when the view is loading data - means, you may show an appropriate loading state.

Finally, your load function may fail. It also might be a good idea to show an error to the user.

So, let's gather this together and let's define the enriched view state. First, lets try to identify the various states a view may be in:

DirectorView view state:

enum ViewState {
    case initial
    case loading
    case error
    case idle
}

Well, the view state is not complete yet. We still missing the "data" which should be rendered. In your code, you seem to just need a String, the "directorName":

enum ViewState {
    case initial
    case loading(name: String?)
    case error(name: String?, error: Error)
    case idle(name: String?)

    init() { self = .initial }
}

Applying View Updates

If I am not missing something, you should be able to nicely draw any state of your Director View now from just taking the view state from above.

The good thing also with using an Enum, it's basically impossible to set a state which is not legally covered by the view state - means, your view will never be in some "illegal" state.

How the view is rendering this, is of course the ViewController and its view's part. Usually, you will implement a function in your view controller, like:

func update(new: ViewState, old: ViewState) -> Void

You can imagine easily, I guess, that you can calculate a diff of new and old, and then apply nice animations for the changed content.

After the update function returns, you set the new view state in the view controller. Remember, that the ViewController maintains a copy of view state (UIKit only).

Subtleties

In UIKit, you may face a couple subtle issues here: since animations may be asynchronous you should take care of that. So, update may have a completion handler and update functions should not overlap! Well, in general, but maybe not always... I won't go into details here, though.

View Model

As said in the beginning, your ViewModel generates the view state and provides functions which perform "actions" which get triggered by the view or elsewhere.

Given you can use Combine a simple View Model may look like this:

final class ViewModel: ObservableObject {
    @Published private(set) var viewState: ViewState = .init()

    func load() { ... }
    func dismissError() { ... }
}

What you have to do, is to move your fetch function to the view model, basically somehow into the new load function:

You execute the "load" event taking the current view state into account, and generate a new view state.

There are various ways one can implement this. I personally prefer to use a FSM. Here, I use a more "free style" way to implement it:

func load() {
    guard case .loading != self.viewState else {
        return 
    }    
    guard case .error != self.viewState else {
        return
    }
    let prevName: String? = self.viewState.name // defined in an extension
    newViewState = .loading(name: prevName)
    
    do {
        let director = try await fetchDirector()
        self.viewState = .idle(name: director.name)
    } catch {
        self.viewState = .error(name: prevName, error: error)
    } 
} 

So, this should give you an idea how to implement the view model's action functions. It's not complete yet, though. You need to implement the dismissError otherwise, you stuck in your error state.

What's next you can consider to refactor:

load is just handling a certain event, namely load, we might now consider a more general function perform that handles all events:

static func perform(old: ViewState, event: Event) 

where Event might be an Enum:

enum Event {
    case load 
    case dismissError
}

Here, you switched to an "event driven approach". With hat technique, you can easily implement your "logic" with Swift Combine.

More Refactoring Using "Side Effects"

Note, this is more advanced, and I only give some ideas. Doing research in this area is recommended.

A side effect is simply everything that access the "outer world" - where the inner world is your view model. So, reading a date, calling a network endpoint, printing to the console, getting a random Int, this all are side effects.

It would be nice to have a way to configure the concrete side effects, because this tremendously simplifies testing.

So, this might require some more changes in the perform function: it should become pure and synchronous and most importantly, it should not start async tasks like in the former implementation, but rather do it in an "indirect way":

func update(old: ViewState, event: Event) -> (ViewState, Command)

Where "Command" is a representation of a "Side Effect", which get started later in the processing and the task's (eventual) result, an Event variable, gets fed into the update function again.

That also means in your case, you would need a new Event case which represents the result or the "Output" of your fetch function:

enum Event {
    // Actions:
    case load 
    case dismissError

    // Outputs from "Side Effects":
    case data(Director)
    case error(error: Error)
}

Since update now only depends on its input parameters and does not perform any side effects, it becomes super easy to test (that way, you made the function update a pure function). Note, this update function contains the whole presentation logic of the view model.

Usually, your View Model gets a list of concrete Side Effects when it is initialised. So, using "Side Effects" you can use this as a "Dependency Injection Point", where you can inject Mocks for testing.