-
Notifications
You must be signed in to change notification settings - Fork 0
feat(allProject): add mvvm, network services #4
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Папку xcuserdata стоит игнорировать, кладя в .gitignore. В ней хранится информация о состоянии проекта (какой файл открыт, какой таргет выбран и т.п.), брейкпоинты и схемы для текущего пользователя Xcode.
Про gitignore можно прочитать вот тут
|
|
||
| @main | ||
| struct ScheduleTrackerApp: App { | ||
| @StateObject private var services = try! APIServicesContainer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше не использовать force операции
| @Published var tickets: [TicketModel] = [] | ||
| @Published var isLoading = false | ||
| @Published var errorType: AppErrorType = .none | ||
| @Published var showTransfers: Bool? = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
= nil можно опускать
| struct CarrierInfoView: View { | ||
| @Environment(\.dismiss) var dismiss | ||
| let carrier: TicketModel | ||
| @StateObject var viewModel: CarrierInfoViewModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StateObject (как и State) должен быть приватным. Если мы модель извне передаем, то стоит ObservedObject использовать
| Text("E-mail") | ||
| .font(.custom("SFPro-Regular", size: 16)) | ||
| if viewModel.isLoading { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Везде, где это возможно (если layout при этом не изменяется), лучше использовать opacity вместо if для сокрытия View. Вот хороший пример с ссылками на WWDC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Непонятное замечание. Что имеете в виду?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В SwiftUI, если нужно просто скрыть View, лучше использовать .opacity, а не if-else.
Когда ты пишешь if-else, SwiftUI во время компиляции создаёт две разные версии View: одну при true, другую при false. Это значит, что в иерархии появляется больше элементов, между которыми нужно переключаться во время работы - это дополнительные ресурсы.
А если использовать .opacity, то View всегда остаётся в иерархии одна. Она просто становится невидимой, и не тратятся ресурсы на перестройку структуры. Это проще и производительнее.
Это более продвинутая информация. Об этом подробнее можно по ссылке выше почитать или есть хорошая книга по SwiftUI на английском. В интернете можно бесплатную pdf найти. В ней подробно о механизме работы SwiftUI рассказывается в сравнении с UIKit
|
|
||
| var body: some View { | ||
| ZStack { | ||
| Color("nightOrDayColor").ignoresSafeArea() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше Resource использовать:
Color(.nightOrDay)
| Link("i.lozgkina@yandex.ru", destination: mailURL) | ||
| if let email = carrier.email { | ||
| Text("E-mail") | ||
| .font(.custom("SFPro-Regular", size: 16)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно сделать обертку для кастомных шрифтов, чтобы их было удобнее добавлять и переиспользовать. Например что-то такое:
struct Fonts {
static let sfProBoldFontName = "SFPro-Bold"
static let sfProBold24 = Font.custom(sfProBoldFontName, size: 24)
}| .toolbar { | ||
| ToolbarItem(placement: .navigationBarLeading) { | ||
| Button(action: { dismiss() }) { | ||
| Image("leftChevron") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут тоже лучше Resource использовать:
Image(.leftChevron)
| if viewModel.isLoading { | ||
| Spacer() | ||
| ProgressView() | ||
| Spacer() | ||
| } else if viewModel.errorType == .internet { | ||
| ErrorInternetView() | ||
| } else if viewModel.errorType == .server { | ||
| ErrorServerView() | ||
| } else { | ||
| ScrollView(.vertical, showsIndicators: false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Обработка состояний дублируется в нескольких View. Предлагаю создать отдельную View и поместить в нее общий код, и передавать в эту View отличную View. Будет что-то вроде:
struct StateWrapperView<Content: View>: View {
let isLoading: Bool
let errorType: AppErrorType?
let content: () -> Content
var body: some View {
Group {
if isLoading {
VStack {
Spacer()
ProgressView()
Spacer()
}
} else if errorType == .internet {
ErrorInternetView()
} else if errorType == .server {
ErrorServerView()
} else {
content()
}
}
}
}feat(StateWrapperView) add StateWrapperView in app
|
@NaR0RoW Здравствуй. Если моделям добавлю Sendable и сервисы вместо классов сделаю actor, этого достаточно будет? |
| do { | ||
| return try APIServicesContainer() | ||
| } catch { | ||
| fatalError("Failed to initialize APIServicesContainer: \(error)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По своему опыту хотел бы отметить, что в продакшене не используют fatalError, советую заменить на assertionFailure, тут подробнее
| ZStack { | ||
| Color("nightOrDayColor").ignoresSafeArea() | ||
| VStack (spacing: 16) { | ||
| VStack (alignment: .center, spacing: 16) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alignment: .center можно опустить, т.к. это дефолтное значение
| } | ||
| } | ||
| .frame(width: 220, height: 38, alignment: .leading) | ||
| .frame(width: 38, height: 38, alignment: .center) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут тоже alignment: .center можно опустить
Да, достаточно |
Это в данном случае было формальностью? Ведь все работало и без них, никаких предупреждений не было и все было безопасно на потоках |
Это не формальность, а вложение в масштабируемость и безопасность кода. Сейчас код может работать без Sendable и actor, но:
Простой пример: class NetworkClient {
var token: String = ""
func fetch() async {
// ...
}
}Если кто-то из двух Task параллельно изменяет token - мы ловим data race. |
No description provided.