Code smells, technical debt, стратегии рефакторинга
Рефакторинг — это улучшение структуры кода без изменения поведения. Code review — лучшее время предложить рефакторинг.
# ❌ Дублирование
def get_active_users():
users = User.query.filter_by(is_active=True).all()
return [u.to_dict() for u in users]
def get_inactive_users():
users = User.query.filter_by(is_active=False).all()
return [u.to_dict() for u in users]
def get_admin_users():
users = User.query.filter_by(is_admin=True).all()
return [u.to_dict() for u in users]
# ✅ Рефакторинг: обобщение
def get_users_by_filter(**filters):
users = User.query.filter_by(**filters).all()
return [u.to_dict() for u in users]
# Использование
get_users_by_filter(is_active=True)
get_users_by_filter(is_active=False)
get_users_by_filter(is_admin=True)# ❌ Метод на 100 строк
def process_order(order):
# 20 строк: валидация
if not order.items:
raise EmptyOrderError()
if order.user is None:
raise InvalidUserError()
# ...
# 30 строк: расчёт стоимости
subtotal = sum(item.price * item.qty for item in order.items)
discount = calculate_discount(order.user, subtotal)
tax = calculate_tax(subtotal)
total = subtotal - discount + tax
# ...
# 50 строк: сохранение, уведомления, логирование
# ...
# ✅ Рефакторинг: выделение методов
def process_order(order):
validate_order(order)
subtotal = calculate_subtotal(order)
discount = calculate_discount(order.user, subtotal)
tax = calculate_tax(subtotal)
total = subtotal - discount + tax
save_order(order, total)
send_notifications(order)
log_order(order)# ❌ Класс на 2000 строк
class UserService:
def get_user(self, ...): ...
def create_user(self, ...): ...
def update_user(self, ...): ...
def delete_user(self, ...): ...
def send_email(self, ...): ... # Не своё дело!
def log_action(self, ...): ... # Не своё дело!
def connect_db(self, ...): ... # Не своё дело!
# ✅ Рефакторинг: разделение
class UserRepository: # Только БД
def get_user(self, ...): ...
def save(self, ...): ...
class EmailService: # Только email
def send_welcome(self, ...): ...
class Logger: # Только логирование
def log(self, ...): ...| Вид | Пример | Как исправить |
|---|---|---|
| Код без тестов | Новая фича без тестов | Добавить тесты |
| Устаревшие зависимости | requests==2.20.0 (2018) | Обновить зависимости |
| Закомментированный код | # old_function() | Удалить |
| TODO без срока | # TODO: переделать | Создать задачу, сделать |
| Хардкод | if user_id == 123: | Вынести в конфиг |
| Магические числа | if status == 3: | Константы/enum |
🔴 Критично (сделать сейчас):
- Уязвимости безопасности
- Баги, влияющие на пользователей
- Отсутствие тестов на критичную логику
🟡 Важно (сделать в спринте):
- Дублирование кода
- Длинные методы (> 50 строк)
- Устаревшие зависимости с CVE
🟢 Желательно (бэклог):
- Улучшение именования
- Переименование переменных
- Косметические изменения
Оставляй код чище, чем нашёл.
# Видишь дублирование → выдели функцию
# Видишь длинный метод → разбей
# Видишь магическое число → назови
# ❌ Прошёл мимо
def process():
total = amount * 1.2 # Что за 1.2?
# ... 100 строк кода
# ✅ Улучшил
TAX_RATE = 1.2
def calculate_total(amount):
return amount * TAX_RATE
def process():
total = calculate_total(amount)
# ...Большой рефакторинг (рискованно):
1. Переписать весь модуль
2. Изменить 50 файлов
3. Надеемся, что тесты поймали всё
Маленькие шаги (безопасно):
1. Выделить один метод → тесты → коммит
2. Переименовать переменную → тесты → коммит
3. Переместить класс → тесты → коммит
Ключевая мысль: Рефакторинг — это инвестиция. Предлагайте его в каждом review, но приоритизируйте: безопасность и баги важнее именования.
Вопросы ещё не добавлены
Вопросы для этой подтемы ещё не добавлены.