Skip to content

Sprint_21 - #4

Open
Faifor wants to merge 2 commits into
mainfrom
sprint_21
Open

Sprint_21#4
Faifor wants to merge 2 commits into
mainfrom
sprint_21

Conversation

@Faifor

@Faifor Faifor commented Feb 12, 2026

Copy link
Copy Markdown
Owner

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8c36e9424

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

struct DirectoryAPIStation: Decodable, Sendable {
let title: String
let shortTitle: String?
let transportType: String

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Decode station transport type as optional

The new typed parser makes transport_type mandatory (DirectoryAPIStation.transportType is a non-optional String), but this field is modeled as optional in openapi.yaml (Station.properties.transport_type). If the API returns even one station without that key, decoder.decode(DirectoryAPIResponse.self, from:) throws and the entire directory load fails, so city/station pickers stop populating instead of just skipping incomplete records as before.

Useful? React with 👍 / 👎.

self.allStations = mapped

}
self.allStations = mapped

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Publish station updates from MainActor

StationsPickerViewModel is not @MainActor, and after awaiting the network call this method now assigns self.allStations directly. Non-isolated async functions may resume on a background executor, so this can emit @Published changes off the main thread (the previous version wrapped this write in MainActor.run), which causes SwiftUI thread-safety warnings and can lead to missed or inconsistent UI refreshes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Использовать AI-ботов — это нормально и полезно.
Но на этапе обучения лучше сначала самому разобраться в проблеме и понять её причину.
Боты часто ошибаются или не учитывают архитектурный контекст.
Важно не просто применять правки, а понимать, почему ты их делаешь.

Comment thread Travel/Services/ApiClient.swift Outdated
import OpenAPIURLSession

@MainActor
final class ApiClient {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Сетевой клиент как actor и с методами для каждого сервиса ApiClient сейчас @mainactor final class, не actor

self.allStationsService = AllStationsService(client: client)
}

func getSegments(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Метод getSegments содержит слишком много параметров (11 штук).

Общее правило хорошего кода:

👉 Если у метода больше 4–5 параметров — лучше объединить их в модель.

Почему это важно:

ухудшается читаемость
легко перепутать порядок параметров
сложно поддерживать
нарушается принцип Single Responsibility
сигнатура выглядит громоздко

✅ Правильный вариант — создать модель запроса
1️⃣ Создаем модель


struct SegmentsRequest {
    let apikey: String
    let from: String
    let to: String
    
    var format: String? = nil
    var lang: String? = nil
    var date: String? = nil
    var transportTypes: String? = nil
    var offset: Int? = nil
    var limit: Int? = nil
    var resultTimezone: String? = nil
    var transfers: Bool? = nil
}

2️⃣ Меняем метод

func getSegments(request: SegmentsRequest) async throws -> Segments {
    try await searchService.getSegments(
        apikey: request.apikey,
        from: request.from,
        to: request.to,
        format: request.format,
        lang: request.lang,
        date: request.date,
        transport_types: request.transportTypes,
        offset: request.offset,
        limit: request.limit,
        result_timezone: request.resultTimezone,
        transfers: request.transfers
    )
}

3️⃣ Вызов становится читаемым

let request = SegmentsRequest(
    apikey: key,
    from: "c213",
    to: "c54",
    date: "2026-02-12",
    transportTypes: "train",
    limit: 20
)

let segments = try await api.getSegments(request: request)

@@ -7,94 +7,137 @@

import Foundation

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Когда в файле 300+ строк и 8–10 структур — это сигнал к рефакторингу.

📁 Пример правильной структуры


Directory/
│
├── Models/
│   ├── DirectoryCity.swift
│   ├── DirectoryStation.swift
│
├── APIModels/
│   ├── DirectoryAPIResponse.swift
│   ├── DirectoryCountry.swift
│   ├── DirectoryRegion.swift
│   ├── DirectorySettlement.swift
│   ├── DirectoryAPIStation.swift
│   ├── DirectoryStationCodes.swift
│
├── Services/
│   └── DirectoryService.swift

//

import SwiftUI
@preconcurrency import SwiftUI

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@preconcurrency import SwiftUI здесь не требуется.
Это атрибут для миграции кода на Swift Concurrency и подавления предупреждений, а не стандартная практика.
В данном случае достаточно обычного import SwiftUI.

@@ -18,7 +18,7 @@ struct MainScreenView: View {
@State private var showCityPicker = false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

View в SwiftUI должна быть максимально "тупой" (stateless).
Логику и состояние лучше выносить в ViewModel.

@@ -243,6 +236,7 @@ struct City: Identifiable, Equatable, Hashable {
let name: String
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Сейчас в одном файле находятся:

MainScreenView
CityPickerView
StationsPickerView
SearchPrimaryButton
StoryCardView
вспомогательные модели
extension'ы
ViewModel'и

Это перегружает файл и усложняет навигацию.

📌 Правило

Одно View = один файл.

Даже если проект учебный.

✅ Как правильно организовать

Например:


Main/
│
├── MainScreenView.swift
├── MainViewModel.swift
│
├── Stories/
│   ├── StoriesStripView.swift
│   ├── StoryCardView.swift
│   ├── StoriesPlayerView.swift
│
├── CityPicker/
│   ├── CityPickerView.swift
│   ├── CityPickerViewModel.swift
│
├── StationsPicker/
│   ├── StationsPickerView.swift
│   ├── StationsPickerViewModel.swift
│
├── Components/
│   ├── SearchPrimaryButton.swift
│   ├── RoundedCorner.swift

В командной разработке иначе будет очень сложно ориентироваться и поддерживать код.
Лучше с учебного проекта привыкать писать архитектурно правильно.

self.allStations = mapped

}
self.allStations = mapped

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Использовать AI-ботов — это нормально и полезно.
Но на этапе обучения лучше сначала самому разобраться в проблеме и понять её причину.
Боты часто ошибаются или не учитывают архитектурный контекст.
Важно не просто применять правки, а понимать, почему ты их делаешь.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants