Ревью кода на Swift: 7 ошибок, которые бесят senior-разработчиков

Ревью кода на Swift: как перестать бесить коллег и писать код уровня senior

Вы когда-нибудь отправляли Pull Request, а через час получали 15 комментариев от тимлида? «Force unwrap? Серьезно?», «А где обработка ошибок?», «Этот флоу контроллер висит в памяти уже три дня». Знакомо? Добро пожаловать в мир ревью кода на Swift — место, где даже опытные разработчики иногда чувствуют себя стажерами.

В этой статье я собрал самые частые замечания, которые senior-ы оставляют на code review. Мы разберем реальные примеры кода, антипаттерны и то, как их исправить. Это не скучная теория — это боевой опыт сотен ревью, который поможет вам писать код, который не будет вызывать желания переписать всё с нуля. Поехали!

1. Force Unwrap — первая любовь и главная боль

Начнем с самого банального, но вечного. Force unwrap (!) — это как езда без ремня безопасности. Вроде бы удобно, но одно резкое торможение — и вы в травматологии.

Плохой код (типичный junior):

func getUserName() -> String {
    let name: String? = UserDefaults.standard.string(forKey: "user_name")
    return name!
}

Что скажет senior: «А если в UserDefaults ничего нет? Краш. Всегда. Добавь guard let или defaultValue.»

Хороший код:

func getUserName() -> String {
    guard let name = UserDefaults.standard.string(forKey: "user_name") else {
        return "Гость"
    }
    return name
}

Исключение: Force unwrap допустим только в одном случае — когда вы на 146% уверены, что значение существует. Например, при работе с IBOutlets, которые гарантированно будут проинициализированы. Но даже тут senior посоветует использовать private var label: UILabel! — это конвенция, а не костыль.

2. Магические числа и строки: код не должен быть квестом

Второе, что бросается в глаза на ревью кода на Swift — это магические константы. «Почему именно 0.75? А, это коэффициент анимации... Какой именно?». Если вы пишете числа и строки прямо в коде, вы заставляете коллег играть в детектива.

Плохой код:

if user.age > 18 {
    showAdultContent()
}
button.layer.cornerRadius = 12
let url = "https://api.example.com/v1/users"

Что скажет senior: «18 — это магическое число. Вынеси в константу. URL — в enum. cornerRadius — в стилистическую константу. Код должен читаться как книга, а не как шифр.»

Хороший код:

private enum Constants {
    static let adultAgeLimit = 18
    static let defaultCornerRadius: CGFloat = 12
    static let baseURL = "https://api.example.com/v1"
}

enum APIEndpoint {
    case users
    var url: String {
        switch self {
        case .users: return Constants.baseURL + "/users"
        }
    }
}

if user.age > Constants.adultAgeLimit {
    showAdultContent()
}

3. Memory Leaks: как случайно подвесить приложение

Ревью кода на Swift редко обходится без обсуждения retain cycles. Senior-ы любят эту тему, потому что она показывает, понимаете ли вы, как работает ARC. Самый популярный антипаттерн — захват self в замыкании без weak ссылки.

Плохой код:

class NetworkManager {
    func fetchData(completion: @escaping (Data) -> Void) {
        URLSession.shared.dataTask(with: url) { data, _, _ in
            self.process(data) // self захвачен сильно
            completion(data)
        }
    }
}

Что скажет senior: «Ты держишь сильную ссылку на self внутри замыкания. Пока замыкание живо — объект не удалится. Добавь [weak self] и проверяй на nil. Или используй [unowned self], если точно знаешь, что self переживет замыкание.»

Хороший код:

func fetchData(completion: @escaping (Data) -> Void) {
    URLSession.shared.dataTask(with: url) { [weak self] data, _, _ in
        guard let self = self else { return }
        self.process(data)
        completion(data)
    }.resume()
}

4. Хаотичная архитектура: MVC vs MVVM vs VIPER

О, это любимая тема senior-ов на ревью кода. Они ненавидят, когда в одном проекте смешивают MVC, MVVM и немного VIPER, потому что «так быстрее». Нет, не быстрее. Это хаос.

Типичное замечание на code review: «У тебя в ViewController лежит логика парсинга JSON, URL-запросы и обновление UI. Это Massive View Controller. Вынеси бизнес-логику в отдельный слой.»

Пример рефакторинга (MVVM):

// ViewModel
class UserViewModel {
    private let apiService: APIService
    @Published var users: [User] = []

    func loadUsers() {
        apiService.fetchUsers { [weak self] users in
            self?.users = users
        }
    }
}

// ViewController
class UserViewController: UIViewController {
    private let viewModel = UserViewModel(apiService: APIService())

    override func viewDidLoad() {
        super.viewDidLoad()
        viewModel.$users.sink { [weak self] users in
            self?.tableView.reloadData()
        }
    }
}

Senior-ы ценят, когда ViewController занимается только View, а всё остальное — в сервисах и моделях. Это упрощает тестирование и поддержку.

5. Игнорирование ошибок: try? и пустые catch

Еще один красный флаг на ревью кода на Swift — это try? или пустой catch. Это как сказать: «Мне плевать, что пошло не так, просто продолжай». Senior такое не пропустит.

Плохой код:

do {
    let data = try JSONSerialization.data(withJSONObject: json)
} catch {
    // Игнорируем
}

let data = try? JSONSerialization.data(withJSONObject: json)

Что скажет senior: «Ты теряешь информацию об ошибке. Если что-то упадет, ты не узнаешь почему. Либо обрабатывай ошибку, либо логируй её. try? — только если nil — валидное состояние.»

Хороший код:

do {
    let data = try JSONSerialization.data(withJSONObject: json)
    // работаем с data
} catch {
    print("Ошибка сериализации JSON: \(error.localizedDescription)")
    // Показываем пользователю алерт
}

6. Глобальные синглтоны: зло во плоти

Синглтоны удобны, но senior-ы знают, что они создают скрытые зависимости и усложняют тестирование. Когда вы пишете UserManager.shared в каждом классе — это запах кода.