На что обращаешь внимание при Code Review?

Мой подход к Code Review для Go-разработки

При проведении Code Review я фокусируюсь на нескольких ключевых аспектах, которые можно разделить на технические, архитектурные и процессуальные категории. Вот основные моменты, на которые я обращаю внимание:

1. Качество и читаемость кода

Читаемость и ясность - это основа maintainable кода. В Go особенно важно следовать идиоматическим подходам:

// Плохо: магические числа, непонятные имена
func calc(a, b int) int {
    return a * 100 / b
}

// Хорошо: явные имена, константы
func CalculateDiscountRatio(price, discountPercentage int) int {
    const percentageBase = 100
    return price * discountPercentage / percentageBase
}

Я проверяю:

• Именование переменных, функций, типов (соответствие Go conventions)
• Длину функций (предпочитаю функции до 50 строк)
• Сложность кода (избегаем глубокой вложенности)
• Комментарии - они должны объяснять "почему", а не "что"

2. Соответствие Go-идиомам и best practices

Go имеет свои уникальные идиомы, которые важно соблюдать:

// Идиоматическая обработка ошибок
func ReadConfig(path string) (*Config, error) {
    data, err := os.ReadFile(path)
    if err != nil {
        return nil, fmt.Errorf("reading config: %w", err)
    }
    
    var config Config
    if err := json.Unmarshal(data, &config); err != nil {
        return nil, fmt.Errorf("parsing config: %w", err)
    }
    
    return &config, nil
}

Ключевые проверки:

• Обработка ошибок - всегда ли ошибки проверяются и оборачиваются?
• Использование интерфейсов - обоснованно ли они применяются?
• Работа с concurrent code - правильное использование горутин, каналов, sync примитивов
• Memory management - избегаем утечек, правильно работаем с указателями

3. Архитектурные принципы и дизайн

Архитектурная целостность часто важнее сиюминутных оптимизаций:

// Вместо монолитной функции
func ProcessUserData(userID string) error {
    // 100 строк кода, делающего всё
}

// Разделяем на компоненты с чёткими ответственностями
type UserProcessor struct {
    repo    UserRepository
    validator UserValidator
    notifier UserNotifier
}

func (p *UserProcessor) Process(userID string) error {
    user, err := p.repo.FindByID(userID)
    if err != nil {
        return err
    }
    
    if err := p.validator.Validate(user); err != nil {
        return err
    }
    
    return p.notifier.Notify(user)
}

Я оцениваю:

• Разделение ответственностей (Single Responsibility Principle)
• Связность и зацепление модулей
• Тестируемость дизайна
• Соответствие существующей архитектуре

4. Производительность и эффективность

Для Go особенно важно учитывать производительность:

// Неэффективно: конкатенация в цикле
func buildString(items []string) string {
    var result string
    for _, item := range items {
        result += item // Аллокация на каждой итерации!
    }
    return result
}

// Эффективно: использование strings.Builder
func buildString(items []string) string {
    var builder strings.Builder
    builder.Grow(estimateSize(items)) // Предварительное выделение памяти
    
    for _, item := range items {
        builder.WriteString(item)
    }
    return builder.String()
}

Проверяю:

• Аллокации памяти - можно ли их уменьшить?
• CPU-интенсивные операции - есть ли оптимизации?
• Использование sync.Pool для объектов с коротким временем жизни
• Выбор структур данных (slice vs map, array vs slice)

5. Безопасность и обработка edge cases

Безопасность - это не только про security, но и про robustness:

// Уязвимость: SQL injection
func GetUser(db *sql.DB, name string) (*User, error) {
    query := fmt.Sprintf("SELECT * FROM users WHERE name = '%s'", name)
    // Критическая уязвимость!
}

// Безопасно: использование параметризованных запросов
func GetUser(db *sql.DB, name string) (*User, error) {
    query := "SELECT * FROM users WHERE name = ?"
    row := db.QueryRow(query, name)
    // ...
}

Я обязательно проверяю:

• SQL/NoSQL injection векторы
• Валидацию входных данных
• Обработку граничных условий и corner cases
• Race conditions в concurrent коде

6. Тестирование и поддерживаемость

Качество тестов - индикатор качества кода:

// Плохой тест: хрупкий и непонятный
func TestProcess(t *testing.T) {
    result := Process()
    if result != 42 {
        t.Error("Ошибка")
    }
}

// Хороший тест: читаемый и надёжный
func TestProcess_ReturnsCorrectValue_WhenInputIsValid(t *testing.T) {
    t.Parallel()
    
    // Arrange
    input := validTestInput()
    expected := 42
    
    // Act
    actual, err := Process(input)
    
    // Assert
    require.NoError(t, err)
    assert.Equal(t, expected, actual)
}

Оцениваю:

• Покрытие тестами критической логики
• Читаемость и изолированность тестов
• Наличие интеграционных и e2e тестов где необходимо
• Тесты на race conditions для concurrent кода

7. Практические рекомендации для ревью

В процессе ревью я следую нескольким важным принципам:

1. Конструктивная критика - предлагаю альтернативы, а не просто указываю на проблемы
2. Приоритизация замечаний - разделяю на critical/major/minor
3. Учёт контекста - понимаю бизнес-ограничения и сроки
4. Обучение и менторинг - использую ревью как возможность научить
5. Автоматизация - там где возможно, полагаюсь на линтеры и статический анализ

Заключение: Эффективный Code Review в Go - это баланс между строгим соблюдением best practices и прагматичным подходом. Я всегда стараюсь понять намерения автора, предложить конкретные улучшения и помнить, что цель ревью - улучшить код, а не просто найти ошибки. Качественный ревью экономит часы отладки и рефакторинга в будущем, поэтому я подхожу к нему с максимальной тщательностью и вниманием к деталям.