MVVM with Combine Tutorial for iOS | raywenderlich.com

In this MVVM with Combine Tutorial, you’ll learn how to get started using the Combine framework along with SwiftUI to build an app using the MVVM pattern


This is a companion discussion topic for the original entry at https://www.raywenderlich.com/4161005-mvvm-with-combine-tutorial-for-ios

There is a bug in the DailyWeatherRowViewModel class in the Starter code. It doesn’t have the proper value defined for the id so if you search for a second city, the display will not update but rather show results for the first city searched. It is correct in the Final code. Please fix this to save developers from going crazy thinking they did something wrong while following the tutorial.

@mattwill09, Thank you for posting this, I was just about to ask if anyone else was facing the issue where the temperatures are not being updated. I suppose this supports the notion that mvvm is more testable because we can apply unit tests to the model. I suppose no value is too simple to be unit tested.

My apologies. I found this bug during my final edit pass and thought I had fixed it in both places but must have grabbed the wrong starter project when I updated the materials. It’s fixed now.

1 Like

I think it might be dangerous for beginners to write this way. I’d like to provide another perspective here.

  1. view model being reference type is against SDK.
    E.g.; struct WeeklyWeatherView: View
    SDK explicitly requires you to use value type to construct a view.
    This is equivalent to bypassing SDK and the protection of immutability.

  2. struct WeeklyWeatherView: View itself is view model.
    Complexity of nested view models might counter any benefits you gain from it.
    You may also make it conform to Codable and decode to it directly without all intermediate structures.

  3. There’s no C in MVVM
    Beginners are easily tempted to do control in view model, again because it lacks immutability.
    It’s very easy to create a massive view model that acts as controller. Then you would need to split it to several view models or even nested ones, creating even more overheads.
    And even if there’s a solution or guideline for that, it is not covered in this tutorial.

  4. it does not do the job better
    If you remove all view models, the codes are cleaner. Immutability forces you to move things out anyway.

Because this tutorial may train the next generation of MVVM developers with SwiftUI, I think it’s important to point out these potential risks for those interested to consider.

1 Like

Hi there :wave:

By SDK I think you are referring to SwiftUI. In its headers it’s quite explicit that entities that confirm with the ‘View’ Protocol should be value types and that’s what we did throughout the tutorial. The ‘WeeklyWeatherView’ is a 'View '( as per its compliance with the ‘View’ protocol) and not a ViewModel. Nesting Views (or composing views), seems to be the mostly accepted approach when wotking with SwiftUI and as such we followed that approach. The ViewModel is MVVM is an interesting layer and there isn’t, to the best of my knowledge, and accepted design. You can see that in one case our ViewModel is the data source that is able to mutate itself, but in others is an immutable entity. I don’t see a particular problem, as long it’s the source of truth and respects the Single Responsibility Principle. You could remove the ViewModels from the code, as you said, but you will need an entity that is able to represent it. The tutorial would also need to change its name to something else (e.g MVC, Viper).

Thank you for your comment.

Hi,

I think you did fine work on tutorial. My concern is that MVVM doesn’t work well with SwiftUI, especially for beginners.

I’ll clarify some of the points you mentioned.

First, sorry for confusion. I meant to say class WeeklyWeatherViewModel: ObservableObject is a reference type view model, whereas struct WeeklyWeatherView: View is value type.

The fundamental difference of opinion is that is struct WeeklyWeatherView: View a view or a model?
You think it’s a view because it conforms to View. I think it’s a model that can be used to construct a view.
I’d argue that a view like Button needs to be reference type because it has mutable properties like title, color, 
etc. So the fact that WeeklyWeatherView is a value type makes it a model. A naming of struct WeeklyWeather: View would make this model-view map relation more clear.

For nesting views, I think there’s a difference between nesting views and nesting view models. Nesting views are common of course. But your view models are not views. They don’t even conform to View. And the worst of all, they may contain side effects like networking. In old days, UIViewController clearly cannot be used as view model so there’s no risk of nesting view model (at least not immediately).

But SDK changes. From what I can tell, SwiftUI has some of the MVVM features built-in. For argument’s sake let’s assume struct WeeklyWeather: View is indeed a view model. Now do you create a new view model or just straight up use it?

As for nested view model being “accepted”. I’d say from my experience it often makes the code more complicated, so not accepted by me personally. I’m sure it has some merits, and the point I’m trying to make here it to provide an alternative. Developers should make their own calls.

Now for class WeeklyWeatherViewModel: ObservableObject being a reference type. Yes you can use access control or other means for better safety. But the point I’m making is that it’s dangerous for beginners because they don’t know they need to. Even experienced developers would often abuse reference type.

I think SwiftUI doesn’t care if you have “good intentions”. Immutable type and compiler check are the safest bets, which is why I say view model should be value type and that type is required by SwiftUI by default. These are design patterns accepted by SwiftUI itself.

Lastly, as you said, the tutorial may have to change name if you remove all view models.
I’m not asking you to. A comparison is a good thing. But I do think MVVM may not be as “good” as before SwiftUI, and this needs to be pointed out.

Well, it’s not like a lot of people would read comments anyway :grinning:
So I think I’ll leave the discussion here. People who read this can make up their own minds.

Thanks

Hi there!

I think there needs to be a separation between what we at RW think is a good approach and what Apple as set for us. Views having value semantics falls under the latter and this is quite explicit in their headers. And exactly because of this, I don’t think it’s constructive to focus the conversation on the nature of the Views. I also don’t think it’s worth, from a consumer point of view, comparing a UIKit View with a SwiftUI View, since they follow different paradigms and clearly are/were design differently. Regarding VM’s nesting, YMMV. Personally I don’t see anything particularly wrong with it, but in any case IIRC we didn’t do so in the sample code.

For argument’s sake let’s assume struct WeeklyWeather: View is indeed a view model. Now do you create a new view model or just straight up use it?

I am not too sure why you are making this argument, since it’s not something we have done.

Now for class WeeklyWeatherViewModel: ObservableObject being a reference type. Yes you can use access control or other means for better safety. But the point I’m making is that it’s dangerous for beginners because they don’t know they need to. Even experienced developers would often abuse reference type.

The ObservableObject protocol forces you to use a reference type, since it has a : AnyObject “contraint”, so your ViewModel (or whatever you want to call it) will be a reference type.

Nomenclature aside, I would say that both the tutorial and the code are actually pretty conservative, since we just followed what Apple has laid for us. As mentioned in the article the most contemptuous point, IMO, is how the routing is being done.

Thank you! :smile:

Hi, thanks for the excellent tutorial. I’m following closely, but when I run the “Finished” sample, I am not seeing any results for the weather. The personal app I built following this also doesn’t seem to make an API call. Any idea what’s going wrong?

In the scene delegate I see that the stuff is initialized like this:

let fetcher = WeatherFetcher()
let viewModel = WeeklyWeatherViewModel(weatherFetcher: fetcher)
let weeklyView = WeeklyWeatherView(viewModel: viewModel)

Does WeatherFetcher need to be initialized with anything?

@rperes Can you please help with this when you get a chance? Thank you - much appreciated! :]

Thank you very much for this. It is extremely helpful. The one thing I am still uncomfortable with is having to import SwiftUI into the view models. It seems like view models shouldn’t need to know about SwiftUI. But I don’t see any obvious way around that. Does anyone have any thoughts on this?

If anyone is interested, I have started an app using your tutorial as a guideline. I have also added CoreData and a lot of tests. It is still a work in progress but I would appreciate any feedback. Thanks. It is here: GitHub - jodihum/needledrop: Part of an app using SwiftUI, Combine, and CoreData, using MVVM architecture. With lots of tests.

While I appreciate the work that went into this tutorial, I feel like it completely loses its purpose. It’s far too overly complicated for something that sounded like it was going to be a tutorial on how to work with Combine and MVVM.

Starting small then building up to the more advanced features in my opinion would be a much better approach. This tutorial feels like it “starts on step 27” so to speak.

2 Likes

+1
it indeed loses its focus and purpose

There is an issue with the current code in the WeeklyWeatherViewModel.
It seems that if you keep a reference to the subscription generated in the init method for `$city, it works as expected
 something like:

  // reference! 
  var cancellable:AnyCancellable? 

  // 1
  init(
    weatherFetcher: WeatherFetchable,
    scheduler: DispatchQueue = DispatchQueue(label: "WeatherViewModel")
  ) {
    self.weatherFetcher = weatherFetcher
   
    // 2
    cancellable = $city
      .dropFirst(1)
3 Likes

Is the use of weak needed? Since the life cycle of the Cancellable is dependent on the view model life cycle using the store(in:) call at the end?

This works great! Thank you!

I think there is a problem with the WeeklyWeatherViewModel as it doesn’t capture the changes of the @Published var city

The result of the .sink needs to be stored instead of discarding the return value _ = $city

    init(weatherFetcher: WeatherFetchable, scheduler: DispatchQueue = DispatchQueue(label: "WeatherViewModel")) {
        self.weatherFetcher = weatherFetcher
        $city
            .dropFirst()
            .debounce(for: .seconds(0.5), scheduler: scheduler)
            .sink(receiveValue: fetchWeather(forCity:))
             // this is missing
            .store(in: &disposables)
    }

@rperes Do you have any feedback about this? Thank you - much appreciated! :]

Neither the starter (tutorial completed) nor the final project return any weather data for me.

I can confirm that the subscriber needs to be stored for the app to work. In fact I was confused that why is it even working, why is the city’s subscriber not getting deallocated. Looks like there was some issue in Xcode 11.1. The app works fine in Xcode 11.1 but we don’t see any results in Xcode 11.3.1 because subscriber is getting deallocated, which is expected, unless I am missing something.