Async await ugly design

Evolution Discussion
,
1

I was using asyn await for my uikit code and it's ugly

func viewLoaded() {
    task?.cancel()
    task = Task { [weak self] in
        guard let self else { return }
        await self.view?.showLoading()
        do {
            let data = try await service.fetchData()
            await self.view?.show(data)
        } catch {
            await self.view?.show(error)
        }
    }
}

would it be more readable if it was like this

func viewLoaded() {
    view?.showLoading()
    do {
        let data = try await service.fetchData()
        view?.show(data)
    } catch {
        view?.show(error)
    }
}

Note that all methods of view are marked with @MainActor

3

showLoading does not have to be in the Task.

func viewLoaded() {
    task?.cancel()
    view?.showLoading()
    task = Task { [weak self] in
        guard let self else { return }
        do {
            let data = try await service.fetchData()
            await self.view?.show(data)
        } catch {
            await self.view?.show(error)
        }
    }
}
4

You can hide that complexity into a base view controller class to make a nicer API for every day usage.

Are you sure you want show(data) and show(error) to be async? Normally things like those are normal synchronous functions.

On a different note – I don't think your usage of weak self is great – typically viewDidLoad happens very early in the view (viewController) life cycle, say 0.01 after the view controller is created, so at the time of the guard check it would always succeed. Your fetchData however could take quite some time to load data, by which time controller might be already dismissed – but there you have self already captured strongly, so the VC won't be deinited until the fetchData is complete. Might not be a big deal, just be aware that your weak + guard check is mostly useless, could have been a normal strong capture to get to the same result (effectively) as what you have now.

5

I think you can mark your function @MainActor and the task will inherit the main actor and you can drop many awaits. Also I don't think you need to capture self weakly.

@MainActor
func viewLoaded() {
    task?.cancel()
    task = Task {
        self.view.showLoading()
        do {
            let data = try await service.fetchData()
            self.view.show(data)
        } catch {
            self.view.show(error)
        }
    }
}

If that doesn't work, you should also be to mark the Task closure main actor:

Task { @MainActor in
    ...
}
3 Likes
6

+1. Seems like viewLoaded is inside some custom class like presenter which is not part of UIKit (for instance not subclass of UIViewController) and tries to access view properties/methods while not being marked with @MainActor itself.

1 Like
7

Yeah, one of the purposes of await marking is that you’re supposed to think about all the suspensions you’re requesting. If you see a lot of them in a row like this, you should reconsider where the current function is running.

10 Likes