iosswiftswiftuiarchitecture

Is it wrong to use a ViewModel in a simple SwiftUI screen with a list and API call? is there any better approach


I'm building a SwiftUI screen that displays a list of notifications, using a ViewModel to handle state and API calls. The screen is fairly straightforward: it has a header and a paginated notification list that supports pull-to-refresh and infinite scrolling.

Recently, I came across a blog post suggesting that using ViewModels in SwiftUI might be an anti-pattern or unnecessary, especially in simple views(https://developer.apple.com/forums/thread/699003). This made me question whether my current architecture is overkill or misaligned with SwiftUI best practices.

Here is a simplified version of my implementation:

import SwiftUI
@MainActor
class NotificationListViewModel: ObservableObject, APIParamFiledType {
    
    let router: InterCeptor<ProfileEndPoint>
    
    enum NotificationState: Equatable {
        case loading
        case loaded([Notification])
        case paginating([Notification])
        case empty(String)
        case error(String)
    }
    
    @Published var notificationList = [Notification]()
    @Published private(set) var state = NotificationState.loading
    var userModelController: UserModelController
    var pagedObject = PageStruct(indxe: 1, size: 50)
    
    init(router: InterCeptor<ProfileEndPoint>, userModelController: UserModelController) {
        self.router = router
        self.userModelController = userModelController
    }
    
    func loadMoreNotification() async {
        let request = NotificationList.Request(country: userCountry, userInfoId: userInfoId, doctorID: doctorId, pageIndex: pagedObject.index, pageSize: pagedObject.size)
        do {
            let response: NotificationList.Response = try await router.request(endPoint: .notificationList(param: request, authToken: token))
            if notificationList.isEmpty {
                notificationList.append(contentsOf: response.result ?? [])
                if notificationList.isEmpty {
                    state = .empty("No new notifications")
                } else {
                    state = .loaded(notificationList)
                }
            } else {
                notificationList.append(contentsOf: response.result ?? [])
                state = .paginating(notificationList)
            }
            pagedObject.totalCount = response.totalCount
           
        } catch let error {
            state = .error(error.localizedDescription)
        }
    }
    
    func resetNotification() async {
        notificationList.removeAll()
        pagedObject.resetPageIndex()
        await loadMoreNotification()
    }
    
    func shouldLoadMore(currentOffset: Int) async {
        if pagedObject.shouldLoadMore && currentOffset == notificationList.count - 1 {
            pagedObject.increasePageIndex()
            await loadMoreNotification()
        }
    }
}

here is my view

import SwiftUI
import JIGUIKit

struct NotificationListView: View {
    
    var backButtonClick: (() -> Void)?
    @ObservedObject var viewModel: NotificationListViewModel
    
    var body: some View {
        ZStack {
            GradientBlueView()
                .ignoresSafeArea()
            VStack(spacing: 0) {
                headerView
                contentView
            }.frame(maxHeight: .infinity, alignment: .top).onAppear {
                UIRefreshControl.appearance().tintColor = .white
                UIApplication.shared.applicationIconBadgeNumber = 0
                Task {
                    await viewModel.loadMoreNotification()
                }
            }
            .ignoresSafeArea(.container, edges: [.top, .leading, .trailing])
        }
    }
    
    private var headerView: some View {
        HeaderViewWrapper(backButtonClick: backButtonClick)
            .frame(height: 100)
    }
    
    @ViewBuilder
    private var contentView: some View {
        switch viewModel.state {
        case .loading:
            initalLoadingView
        case .loaded(let notifications), .paginating(let notifications):
            List {
                showList(notifications: notifications)
                if case .paginating = viewModel.state {
                    loaderView.listRowBackground(Color.clear)
                }
            }.refreshable(action: {
                Task {
                    await viewModel.resetNotification()
                }
            })
            .padding(.horizontal, 16)
            .listStyle(.plain)
            .applyScrollIndicatorHiddenIfAvailable()
            
        case .empty(let emptyNotification), .error(let emptyNotification):
            showError(error: emptyNotification)
        }
    }
    
    private var initalLoadingView: some View {
        VStack {
            Spacer()
            loaderView
            Spacer()
        }
    }
    
    private var loaderView: some View {
        HStack {
            Spacer()
            BallPulseSync(ballSize: 20, ballColor: .buttonBackground)
            Spacer()
        }.frame(height: 100)
    }
    
    func showError(error: String) -> some View {
        VStack {
            Spacer()
            HStack {
                Spacer()
                Text(error).font(.headline).foregroundStyle(Color.white)
                Spacer()
            }
            Spacer()
        }
    }
    
    func showList(notifications: [Notification]) -> some View {
        ForEach(notifications.indices, id: \.self) { index in
            let notification = notifications[index]
            NotificationRow(notification: notification)
                .padding(.vertical, 10)
                .listRowInsets(EdgeInsets())
                .listRowSeparator(.hidden)
                .listRowBackground(Color.clear)
                .onAppear {
                    Task {
                        await viewModel.shouldLoadMore(currentOffset: index)
                    }
                }
        }
    }
}

I experimented with managing all logic inside the View itself, including state management and API calls, without using a separate ViewModel. However, the view became cluttered and harder to test, so I moved the logic into a dedicated ObservableObject ViewModel for better separation of concerns.


Solution

  • I would say it is misaligned. The whole reason Swift and SwiftUI was designed around struct (aka values) instead of objects is to prevent the shared mutable state problem which causes inconsistencies. E.g. when one part of UI shows one thing and another shows it out of date, that's caused by using objects for the view data and the view didn't update. So attempting to use custom view model objects is like starting off on the wrong foot. If that isn't enough, Apple flat out said in a WWDC talk, "View structs should be your primary encapsulation technique", that means put your view data in there not in your own objects.

    Instead of your own view model object try learning @State. When the State is set, body is called and then you generate all the View structs that describe how you want to show the data. These structs are super lightweight, similar to an int you wouldn't think twice about. SwiftUI will diff these structs and only if there is a difference will it create/update/delete the actual drawing on screen. So we rely on Apple's code to update the screen consistently and all we have to do is init all the View structs which is basically trivial computation-wise. If you attempt to put your own objects on top of the View structs then you are asking for a world of pain because you might updating data at the right time and body might not get called.

    @State is actually like a virtual view model object. When the struct is re-init, SwiftUI gives it back the value it had last time. So it has the benefits of a view model object but its in a struct.

    Async/await replaces closures with sort of an object lifetime for the duration of the logic. The way to use async/await with a lifetime tied to a view is .task. async/await funcs are designed to return data. If you have async func and don't have a return then it is probably wrong.

    So I think a design more aligned with SwiftUI would be to use @State for your view data and .task for your view logic, e.g.

    @State result: Result<String, Error>?
    
    ...
    ZStack {
        if let result {
            switch(result) {
                case .success(text):
                ...
            }
        }
    }
    .task(id: value) {
        do {
            let text = try controller.fetch(using: value) // usually controller is an @Environment struct so it can be tested and mocked for previews
            result = .success(test)
        }
        catch {
            result = .failure(error)
        } 
    }