Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update actions #230

Merged
merged 5 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ import SwiftUI
import LogCore
import UICore

// MARK: Action
// MARK: Actions

enum SakatsuInputScreenAction {
case onSaveButtonClick
case onErrorAlertDismiss
case onCancelButtonClick
}

enum SakatsuInputScreenAsyncAction {
}

// MARK: - View

struct SakatsuInputScreen: View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import SwiftUI
import Algorithms
import SakatsuData

// MARK: Action
// MARK: Actions

enum SakatsuInputViewAction {
case onAddNewSaunaSetButtonClick
Expand All @@ -23,6 +23,9 @@ enum SakatsuInputViewAction {
case onAddNewTemperatureButtonClick
}

enum SakatsuInputViewAsyncAction {
}

// MARK: - View

struct SakatsuInputView: View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import LogCore

// MARK: UI state

struct SakatsuInputUiState {
struct SakatsuInputUiState: Sendable {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SakatsuSakatsuInputErrorSendable に準拠していないのに、 SakatsuInputUiStateSendable に準拠できるのってどういう理由でしたっけ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

○○UiStateSendable に準拠させるのは、メインアクター外からゲットしたいためです。
セットは Task { @MainActor in ... } 内で行えばいいです。

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SakatsuとSakatsuInputErrorすべてがSendableプロトコルに準拠しているなら、struct SakatsuInputUiStateをSendableプロトコルに準拠してもコンパイルエラーが起こらないので、SakatsuとSakatsuInputErrorもSendableプロトコルに暗黙的に準拠していると考えられます!Takeshiさんの本参考にさせてもらいました。
https://github.com/SatoTakeshiX/first-step-swift-concurrency/blob/main/try-concurrency.playground/Pages/5-1-sendable.xcplaygroundpage/Contents.swift

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public出ないなら暗黙的に準拠できると書いてありました。
なんでpublicだったら暗黙的に準拠できているかまではわからなかったので、また教えてください!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

それだー! ありがと 🙏
暗黙的に準拠しているなら Sendable を明示的に付けるのをやめました! c9b45fd

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public だと暗黙的に準拠しない理由は…わからないw
ライブラリとして提供する場合にわかりづらいからとか、、?

var sakatsu: Sakatsu
var sakatsuInputError: SakatsuInputError?
}
Expand All @@ -14,13 +14,18 @@ enum SakatsuEditMode {
case edit(sakatsu: Sakatsu)
}

// MARK: - Action
// MARK: - Actions

enum SakatsuInputAction {
case screen(_ action: SakatsuInputScreenAction)
case view(_ action: SakatsuInputViewAction)
}

enum SakatsuInputAsyncAction {
case screen(_ asyncAction: SakatsuInputScreenAsyncAction)
case view(_ asyncAction: SakatsuInputViewAsyncAction)
}

// MARK: - Error

enum SakatsuInputError: LocalizedError {
Expand Down Expand Up @@ -78,8 +83,7 @@ final class SakatsuInputViewModel: ObservableObject {
self.validator = validator
}

// swiftlint:disable:next cyclomatic_complexity function_body_length
func send(_ action: SakatsuInputAction) {
func send(_ action: SakatsuInputAction) { // swiftlint:disable:this cyclomatic_complexity function_body_length
let message = "\(#function) action: \(action)"
Logger.standard.debug("\(message, privacy: .public)")

Expand Down Expand Up @@ -207,4 +211,20 @@ final class SakatsuInputViewModel: ObservableObject {
}
}
}

nonisolated
func sendAsync(_ asyncAction: SakatsuInputAsyncAction) async {
let message = "\(#function) asyncAction: \(asyncAction)"
Logger.standard.debug("\(message, privacy: .public)")

switch asyncAction {
case let .screen(screenAsyncAction):
switch screenAsyncAction {
}

case let .view(viewAsyncAction):
switch viewAsyncAction {
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import SakatsuData
import LogCore
import UICore

// MARK: Action
// MARK: Actions

enum SakatsuListScreenAction {
case onAddButtonClick
Expand All @@ -16,6 +16,10 @@ enum SakatsuListScreenAction {
case onSettingsButtonClick
}

enum SakatsuListScreenAsyncAction {
case task
}

// MARK: - View

package struct SakatsuListScreen: View {
Expand Down Expand Up @@ -63,6 +67,9 @@ package struct SakatsuListScreen: View {
error: viewModel.uiState.sakatsuListError,
onDismiss: { viewModel.send(.screen(.onErrorAlertDismiss)) }
)
.task {
await viewModel.sendAsync(.screen(.task))
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send(_:) メソッドは同期処理用なので、非同期用に sendAsync(_:) メソッドと ○○AsyncAction を用意しました。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

今回は task のみですが、 ○○AsyncAction は task 以外にも refreshable などが思いつきます。

}
}

@MainActor
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import SwiftUI
import SakatsuData

// MARK: Action
// MARK: Actions

enum SakatsuListViewAction {
case onCopySakatsuTextButtonClick(sakatsuIndex: Int)
case onEditButtonClick(sakatsuIndex: Int)
case onDelete(_ offsets: IndexSet)
}

enum SakatsuListViewAsyncAction {
}

// MARK: - View

struct SakatsuListView: View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import LogCore

// MARK: UI state

struct SakatsuListUiState {
struct SakatsuListUiState: Sendable {
var sakatsus: [Sakatsu] = []
var selectedSakatsu: Sakatsu?
var sakatsuText: String?
Expand All @@ -31,13 +31,18 @@ struct SakatsuListUiState {
}
}

// MARK: - Action
// MARK: - Actions

enum SakatsuListAction {
case screen(_ action: SakatsuListScreenAction)
case view(_ action: SakatsuListViewAction)
}

enum SakatsuListAsyncAction {
case screen(_ asyncAction: SakatsuListScreenAsyncAction)
case view(_ asyncAction: SakatsuListViewAsyncAction)
}

// MARK: - Error

enum SakatsuListError: LocalizedError {
Expand Down Expand Up @@ -68,8 +73,6 @@ final class SakatsuListViewModel: ObservableObject {
self.uiState = SakatsuListUiState()
self.onSettingsButtonClick = onSettingsButtonClick
self.repository = repository

refreshSakatsus()
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

リフレッシュ処理を ViewModel の init に書くとキャンセルできないため、task へ移動しました。

}

func send(_ action: SakatsuListAction) { // swiftlint:disable:this cyclomatic_complexity
Expand All @@ -88,7 +91,9 @@ final class SakatsuListViewModel: ObservableObject {

case .onSakatsuSave:
uiState.shouldShowInputScreen = false
refreshSakatsus()
Task {
await refreshSakatsus()
}

case .onInputScreenCancelButtonClick:
uiState.shouldShowInputScreen = false
Expand Down Expand Up @@ -133,16 +138,45 @@ final class SakatsuListViewModel: ObservableObject {
}
}
}

nonisolated
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ViewModel 全体に @MainActor を付けているため、メインアクター外で非同期メソッドを実行するために nonisolated を付けています。
付けないと処理が重い場合に画面が固まってしまいます。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nonisolated を付けるより、ViewModel 全体から @MainActor を外し、必要なプロパティやメソッドのみに @MainActor を付けるほうがいいでしょうか?
意見募集しています!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あとリフレッシュ中に画面を固まらせない場合、インジケータを表示したり、ビューを disabled にしたりしたほうがいいでしょうか?
こちらも意見募集しています!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

非同期メソッドの実装次第ですが、 nonisolated を付けなくてもメインアクター外で実行できたので消しました…。 c9b45fd

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

リフレッシュ中は特に制御しなくてもよさそうです。
ただ保存時はビューを disabled にしたほうがいいと感じました。

func sendAsync(_ asyncAction: SakatsuListAsyncAction) async {
let message = "\(#function) asyncAction: \(asyncAction)"
Logger.standard.debug("\(message, privacy: .public)")

switch asyncAction {
case let .screen(screenAsyncAction):
switch screenAsyncAction {
case .task:
await refreshSakatsus()
}

case let .view(viewAsyncAction):
switch viewAsyncAction {
}
}
}
}

// MARK: - Privates

private extension SakatsuListViewModel {
func refreshSakatsus() {
nonisolated
func refreshSakatsus() async {
do {
uiState.sakatsus = try repository.sakatsus()
#if DEBUG
try await Task.sleep(for: .seconds(3))
uhooi marked this conversation as resolved.
Show resolved Hide resolved
#endif
let sakatsus = try repository.sakatsus()
Task { @MainActor in
uiState.sakatsus = sakatsus
uhooi marked this conversation as resolved.
Show resolved Hide resolved
}
} catch is CancellationError {
return
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

キャンセルはエラー扱いにしません。

} catch {
uiState.sakatsuListError = .sakatsuFetchFailed(localizedDescription: error.localizedDescription)
Task { @MainActor in
uiState.sakatsuListError = .sakatsuFetchFailed(localizedDescription: error.localizedDescription)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ import SwiftUI
import LogCore
import UICore

// MARK: Action
// MARK: Actions

enum SettingsScreenAction {
case onErrorAlertDismiss
}

enum SettingsScreenAsyncAction {
case task
}

// MARK: - View

package struct SettingsScreen: View {
Expand All @@ -25,6 +29,9 @@ package struct SettingsScreen: View {
error: viewModel.uiState.settingsError,
onDismiss: { viewModel.send(.screen(.onErrorAlertDismiss)) }
)
.task {
await viewModel.sendAsync(.screen(.task))
}
}

@MainActor
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import SwiftUI
import SakatsuData

// MARK: Action
// MARK: Actions

enum SettingsViewAction {
case onDefaultSaunaTimeChange(_ defaultSaunaTime: TimeInterval?)
Expand All @@ -10,6 +10,9 @@ enum SettingsViewAction {
case onLicensesButtonClick
}

enum SettingsViewAsyncAction {
}

// MARK: - View

struct SettingsView: View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,23 @@ import LogCore

// MARK: UI state

struct SettingsUiState {
struct SettingsUiState: Sendable {
var defaultSaunaTimes: DefaultSaunaTimes = .init()
var settingsError: SettingsError?
}

// MARK: - Action
// MARK: - Actions

enum SettingsAction {
case screen(_ action: SettingsScreenAction)
case view(_ action: SettingsViewAction)
}

enum SettingsAsyncAction {
case screen(_ asyncAction: SettingsScreenAsyncAction)
case view(_ asyncAction: SettingsViewAsyncAction)
}

// MARK: - Error

enum SettingsError: LocalizedError {
Expand Down Expand Up @@ -49,8 +54,6 @@ final class SettingsViewModel: ObservableObject {
self.onLicensesButtonClick = onLicensesButtonClick
self.repository = repository
self.validator = validator

refreshDefaultSaunaTimes()
}

func send(_ action: SettingsAction) {
Expand Down Expand Up @@ -92,16 +95,45 @@ final class SettingsViewModel: ObservableObject {
}
}
}

nonisolated
func sendAsync(_ asyncAction: SettingsAsyncAction) async {
let message = "\(#function) asyncAction: \(asyncAction)"
Logger.standard.debug("\(message, privacy: .public)")

switch asyncAction {
case let .screen(screenAsyncAction):
switch screenAsyncAction {
case .task:
await refreshDefaultSaunaTimes()
}

case let .view(viewAsyncAction):
switch viewAsyncAction {
}
}
}
}

// MARK: - Privates

private extension SettingsViewModel {
func refreshDefaultSaunaTimes() {
nonisolated
func refreshDefaultSaunaTimes() async {
do {
uiState.defaultSaunaTimes = try repository.defaultSaunaTimes()
#if DEBUG
try await Task.sleep(for: .seconds(3))
#endif
let defaultSaunaTimes = try repository.defaultSaunaTimes()
Task { @MainActor in
uiState.defaultSaunaTimes = defaultSaunaTimes
}
} catch is CancellationError {
return
} catch {
uiState.settingsError = .defaultSaunaSetFetchFailed(localizedDescription: error.localizedDescription)
Task { @MainActor in
uiState.settingsError = .defaultSaunaSetFetchFailed(localizedDescription: error.localizedDescription)
}
}
}

Expand Down