Как надо делать рефакторинг?

Методология рефакторинга

Рефакторинг — это улучшение внутренней структуры кода БЕЗ изменения его внешнего поведения. Неправильный рефакторинг может сломать функциональность, поэтому нужна строгая методология.

Золотое правило: Red-Green-Refactor (TDD цикл)

1. RED — напишешь падающий тест
2. GREEN — напишешь минимальный код для прохождения
3. REFACTOR — улучшаешь код, сохраняя тесты зелёными

Пример рефакторинга с тестами

ДО: Плохой код (God Function)

# api/services/user_service.py
from sqlalchemy.orm import Session
from app.models import User
import re
from datetime import datetime, timedelta

def process_user_registration(data: dict, session: Session):
    """Функция делает ВСЁ: валидация, регистрация, отправка письма"""
    
    # Валидация email
    if not data.get('email'):
        raise ValueError('Email required')
    
    if not re.match(r'^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$', data['email']):
        raise ValueError('Invalid email')
    
    # Проверка пароля
    if len(data.get('password', '')) < 8:
        raise ValueError('Password too short')
    
    if not any(c.isupper() for c in data['password']):
        raise ValueError('Password must contain uppercase')
    
    # Проверка существования пользователя
    existing = session.query(User).filter(User.email == data['email']).first()
    if existing:
        raise ValueError('User already exists')
    
    # Создание пользователя
    user = User(
        email=data['email'],
        password_hash=hash_password(data['password']),
        first_name=data.get('first_name', ''),
        created_at=datetime.utcnow(),
        is_active=True,
        verification_token=generate_token(),
        verification_expires_at=datetime.utcnow() + timedelta(hours=24)
    )
    
    session.add(user)
    session.commit()
    
    # Отправка email
    import smtplib
    server = smtplib.SMTP('smtp.gmail.com', 587)
    server.starttls()
    server.login('noreply@app.com', 'password')
    message = f'Verify your email: {user.verification_token}'
    server.sendmail('noreply@app.com', user.email, message)
    server.close()
    
    return {'user_id': user.id, 'email': user.email}

Проблемы:

• Функция делает слишком много (SRP нарушен)
• Сложно тестировать (зависит от БД и email сервиса)
• Хардкод для email отправки
• Валидация смешана с бизнес-логикой

ШАГ 1: Написать тесты (RED)

# tests/unit/services/test_user_service.py
import pytest
from app.services.user_service import UserRegistrationService
from app.services.email_service import EmailService
from app.validators import EmailValidator, PasswordValidator
from unittest.mock import Mock, patch

class TestUserRegistrationService:
    @pytest.fixture
    def email_service_mock(self):
        return Mock(spec=EmailService)
    
    @pytest.fixture
    def user_repository_mock(self):
        return Mock()
    
    @pytest.fixture
    def service(self, email_service_mock, user_repository_mock):
        return UserRegistrationService(
            user_repository=user_repository_mock,
            email_service=email_service_mock
        )
    
    def test_successful_registration(self, service, user_repository_mock, email_service_mock):
        """Тест успешной регистрации"""
        data = {
            'email': 'john@example.com',
            'password': 'SecurePassword123',
            'first_name': 'John'
        }
        
        user_repository_mock.get_by_email.return_value = None
        user_repository_mock.create.return_value = Mock(id=1, email='john@example.com')
        
        result = service.register(data)
        
        assert result['user_id'] == 1
        assert result['email'] == 'john@example.com'
        user_repository_mock.create.assert_called_once()
        email_service_mock.send_verification.assert_called_once()
    
    def test_email_already_exists(self, service, user_repository_mock):
        """Тест: пользователь уже зарегистрирован"""
        data = {
            'email': 'existing@example.com',
            'password': 'SecurePassword123'
        }
        
        user_repository_mock.get_by_email.return_value = Mock(id=1)
        
        with pytest.raises(ValueError, match='User already exists'):
            service.register(data)
    
    def test_invalid_email(self, service):
        """Тест: невалидный email"""
        data = {
            'email': 'invalid-email',
            'password': 'SecurePassword123'
        }
        
        with pytest.raises(ValueError, match='Invalid email'):
            service.register(data)
    
    def test_weak_password(self, service):
        """Тест: слабый пароль"""
        data = {
            'email': 'john@example.com',
            'password': 'weak123'  # Нет заглавных букв
        }
        
        with pytest.raises(ValueError, match='Password must contain uppercase'):
            service.register(data)

ШАГ 2: Написать минимальный код (GREEN)

# app/validators.py — Validator Pattern
from dataclasses import dataclass
import re
from typing import Optional

@dataclass
class ValidationError:
    field: str
    message: str

class EmailValidator:
    @staticmethod
    def validate(email: str) -> Optional[ValidationError]:
        if not email:
            return ValidationError('email', 'Email required')
        
        pattern = r'^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'
        if not re.match(pattern, email):
            return ValidationError('email', 'Invalid email')
        
        return None

class PasswordValidator:
    MIN_LENGTH = 8
    
    @staticmethod
    def validate(password: str) -> Optional[ValidationError]:
        if len(password) < PasswordValidator.MIN_LENGTH:
            return ValidationError('password', f'Password must be at least {PasswordValidator.MIN_LENGTH} characters')
        
        if not any(c.isupper() for c in password):
            return ValidationError('password', 'Password must contain uppercase')
        
        if not any(c.isdigit() for c in password):
            return ValidationError('password', 'Password must contain digit')
        
        return None

# app/services/email_service.py — Email Service
from abc import ABC, abstractmethod

class EmailService(ABC):
    @abstractmethod
    def send_verification(self, email: str, token: str) -> bool:
        pass

class SmtpEmailService(EmailService):
    def __init__(self, smtp_host: str, smtp_port: int, sender_email: str, sender_password: str):
        self.smtp_host = smtp_host
        self.smtp_port = smtp_port
        self.sender_email = sender_email
        self.sender_password = sender_password
    
    def send_verification(self, email: str, token: str) -> bool:
        import smtplib
        try:
            server = smtplib.SMTP(self.smtp_host, self.smtp_port)
            server.starttls()
            server.login(self.sender_email, self.sender_password)
            message = f'Verify your email: {token}'
            server.sendmail(self.sender_email, email, message)
            server.close()
            return True
        except Exception as e:
            print(f'Failed to send email: {e}')
            return False

# app/repositories/user_repository.py
from sqlalchemy.orm import Session
from app.models import User
from typing import Optional

class UserRepository:
    def __init__(self, session: Session):
        self.session = session
    
    def get_by_email(self, email: str) -> Optional[User]:
        return self.session.query(User).filter(User.email == email).first()
    
    def create(self, email: str, password_hash: str, first_name: str, verification_token: str, verification_expires_at):
        from datetime import datetime
        user = User(
            email=email,
            password_hash=password_hash,
            first_name=first_name,
            verification_token=verification_token,
            verification_expires_at=verification_expires_at,
            is_active=True,
            created_at=datetime.utcnow()
        )
        self.session.add(user)
        self.session.commit()
        self.session.refresh(user)
        return user

# app/services/user_service.py — User Registration Service
from app.repositories.user_repository import UserRepository
from app.services.email_service import EmailService
from app.validators import EmailValidator, PasswordValidator
from app.security import hash_password, generate_token
from datetime import datetime, timedelta

class UserRegistrationService:
    def __init__(self, user_repository: UserRepository, email_service: EmailService):
        self.user_repository = user_repository
        self.email_service = email_service
        self.email_validator = EmailValidator()
        self.password_validator = PasswordValidator()
    
    def register(self, data: dict) -> dict:
        # Валидация
        email_error = self.email_validator.validate(data.get('email', ''))
        if email_error:
            raise ValueError(email_error.message)
        
        password_error = self.password_validator.validate(data.get('password', ''))
        if password_error:
            raise ValueError(password_error.message)
        
        # Проверка существования
        if self.user_repository.get_by_email(data['email']):
            raise ValueError('User already exists')
        
        # Создание пользователя
        password_hash = hash_password(data['password'])
        token = generate_token()
        expires_at = datetime.utcnow() + timedelta(hours=24)
        
        user = self.user_repository.create(
            email=data['email'],
            password_hash=password_hash,
            first_name=data.get('first_name', ''),
            verification_token=token,
            verification_expires_at=expires_at
        )
        
        # Отправка email
        self.email_service.send_verification(user.email, token)
        
        return {'user_id': user.id, 'email': user.email}

ШАГ 3: Рефакторинг (REFACTOR)

Уже сделано в Step 2! Код разбит на слои:

• Validators — валидация
• Services — бизнес-логика
• Repositories — доступ к данным
• Email Service — отправка писем

Приёмы рефакторинга

1. Extract Method

# ДО
def calculate_discount(user_age, membership_years):
    if user_age > 65:
        base_discount = 0.15
    elif membership_years > 5:
        base_discount = 0.1
    else:
        base_discount = 0.05
    
    seasonal_bonus = 0.02 if is_holiday() else 0
    total = base_discount + seasonal_bonus
    return total

# ПОСЛЕ
def calculate_discount(user_age, membership_years):
    base_discount = _get_base_discount(user_age, membership_years)
    seasonal_bonus = _get_seasonal_bonus()
    return base_discount + seasonal_bonus

def _get_base_discount(user_age, membership_years):
    if user_age > 65:
        return 0.15
    elif membership_years > 5:
        return 0.1
    return 0.05

def _get_seasonal_bonus():
    return 0.02 if is_holiday() else 0

2. Replace Magic Numbers with Constants

# ДО
if user_age > 65:
    discount = 0.15
if days_to_expiry < 3:
    send_warning()

# ПОСЛЕ
SENIOR_AGE_THRESHOLD = 65
SENIOR_DISCOUNT = 0.15
EXPIRY_WARNING_DAYS = 3

if user_age > SENIOR_AGE_THRESHOLD:
    discount = SENIOR_DISCOUNT
if days_to_expiry < EXPIRY_WARNING_DAYS:
    send_warning()

3. Simplify Conditional

# ДО
if status == 'active' and role in ['admin', 'moderator']:
    can_approve = True
else:
    can_approve = False

# ПОСЛЕ
can_approve = status == 'active' and role in ['admin', 'moderator']

4. Replace Temp with Query

# ДО
temp = session.query(Order).filter(Order.status == 'completed').all()
for order in temp:
    total += order.amount

# ПОСЛЕ
total = sum(
    order.amount 
    for order in session.query(Order).filter(Order.status == 'completed')
)

Процесс безопасного рефакторинга

1. ✅ Убедись, что есть тесты
2. ✅ Запусти все тесты (GREEN)
3. ✅ Сделай небольшое изменение
4. ✅ Запусти тесты (должны быть GREEN)
5. ❌ Если тесты сломаны — откати изменение
6. ✅ Повторяй шаги 3-5
7. ✅ Commit после каждого успешного рефакторинга

Git workflow при рефакторинге

# Перед началом
git checkout -b refactor/user-service

# Небольшое изменение
git add .
git commit -m "Extract EmailValidator class"

# Запусти тесты
make test

# Если успешно — следующее изменение
git add .
git commit -m "Extract PasswordValidator class"

make test

# После всего рефакторинга
git push origin refactor/user-service

Признаки, что нужен рефакторинг

1. Дублирование кода (DRY нарушен) — extract method
2. Длинные функции (> 20 строк) — разбей на несколько
3. Слишком много параметров (> 3) — используй объект
4. Вложенные циклы — extract method
5. Magic numbers и строки — constants
6. Комплексные условия — extract method
7. Побочные эффекты — изолируй
8. Смешанные ответственности — SRP

Частые ошибки при рефакторинге

# ❌ Ошибка 1: Менять поведение и рефакторить одновременно
def refactor_and_add_feature():
    # Это НЕ рефакторинг!
    pass

# ✅ Правильно: сначала рефакторинг, потом фича
# Commit 1: Рефакторинг
# Commit 2: Новая фича

# ❌ Ошибка 2: Рефакторить без тестов
# Ты не поймешь, что сломал!

# ✅ Правильно: тесты → рефакторинг → тесты должны пройти

# ❌ Ошибка 3: Большие рефакторинги
# Сложно review, много потенциальных ошибок

# ✅ Правильно: маленькие, логические шаги

Лучшие практики

1. Всегда пиши тесты перед рефакторингом
2. Делай маленькие изменения (одна фича per commit)
3. Запускай тесты после каждого изменения
4. Используй IDE для автоматизации (rename, extract)
5. Используй git bisect для поиска проблем
6. Не рефакторь и не добавляй фичи одновременно
7. Code review для важного рефакторинга

Рефакторинг — это не улучшение работы программы, это улучшение структуры кода. Всегда сохраняй функциональность!