Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save defenitionofreal/e5428316fef257e5a54eb62e64a48860 to your computer and use it in GitHub Desktop.
Save defenitionofreal/e5428316fef257e5a54eb62e64a48860 to your computer and use it in GitHub Desktop.
Зачада 1
Комментарии по коду (https://gist.github.com/chepe4pi/3c5c2e4bab87fbf155dfdeaa18e2fc8f):
- Не соблюден PEP8
- Не соблюден прицип единной ответственности, к примреу одна вью не должна выполнять два разных действия
- Бизнес логика не вынесена отдельно
- Нейминг моделей и полей в них затрудняет работу.
Все модели должны называться в единственном числе так как мы перебираем по записи в БД (переименовать модель Costs в Cost и так далее).
Поля в моделях, если возвращают булевый тип, должны начинаться с is_ или has_ (переименовать поле goal if is_goal и так далее)
Поля, которые ссылаются на связь с объектом, не должеы называться orderid, stageid, cityid. (переименовать в order,stage,city)
Поле search в модели Search сбивает и путает, по смыслу логично назвать поле keyword.
- Импорты должны быть разбиты друг от друга. Нужен регламент, что сначала испорты питона, потом джанги, потом библиотек и потом свои.
- фильтр orderid__cityid=None безcмысленый в части if request.user.search.company и вообще странно что cityid фильтруется по company. Проблемы с неймингом.
- фильтр orderid__searchowners__icontains, где icontains не уместно. Предполагаю что searchowners это M2M.
- Не обязательно импортировать модель Favorites. К ней можно добраться через Search.user.favorites_set.
- None и пустая строка '' оба считаются ложными. Достаточно if request.user.search.search вместо request.user.search.search is not None and request.user.search.search != ''
- Много лишних циклов for для заполнения лишних списков. Можно использовать вместо этого фильтр как order_id__in=, а для того чтобы собрать айдишники можно использовать values_list, а для того чтобы собрать только последние элементы можно использовать order_by c distinct.
Таких примеров в коде много. Описывать каждый отдельный случай нету смысла. Вывод, все пустые списки и циклы for можно убрать, если грамотно использовать ORM.
- Можно догадаться, что в шаблонах будут циклы и будут обращения к связанным сущностям. Чтобы решить проблему N+1, используейте select_related и prefetch_related.
Пример рефакторинга с использованием паттерна "репозиторий" и разбиение по слоям для соблюдения чистой архитектуры. Хотя бы частично, насколько django этого позволит.
https://github.com/defenitionofreal/refactoring
Плюсы разбиения по слоям
Паттерн репозиторий.
Основная цель состоит в абстрагировании логики доступа к данным и представлении этой логики как набора методов для работы с данными.
Это позволяет отделить логику приложения от конкретных деталей реализации хранилища данных (например, базы данных).
У такого подхода есть преимущество как централизация запросов, все функции запросов определяются и поддерживаются в одном месте.
Слой валидации.
Этот подход подразумевает создание отдельного класса или модуля для выполнения проверок данных, который можно легко тестировать и повторно использовать.
Основные аспекты подхода:
- Инкапсуляция логики валидации: Вся логика валидации сосредоточена в одном месте, что делает код более читаемым и поддерживаемым.
- Повторное использование: Логику валидации можно использовать в разных частях приложения без дублирования кода.
- Тестируемость: Легко тестировать валидаторы отдельно от остальной логики приложения.
Преимущества подхода:
- Отделение ответственности: Валидаторы сосредотачиваются исключительно на проверке данных, не смешивая логику валидации с другой логикой.
- Чистота кода: Код становится более организованным и легко понимаемым, так как валидация данных вынесена в отдельный слой.
- Гибкость: Легко добавлять или изменять правила валидации без влияния на остальной код.
В первом шаге мы создали репо класс для абстагирования orm запросов и предусмотрели там метода фильтрации (is_existing_phone_number, is_existing_email), которые теперь можем использовать в классе валидации.
Слой сервисов.
Этот подход заключается в создании отдельного слоя в приложении, который содержит бизнес-логику и обеспечивает выполнение операций, связанных с конкретной функциональностью.
Основные аспекты подхода Service Layer:
- Инкапсуляция бизнес-логики: Класс сервиса инкапсулирует бизнес-логику и координирует работу между различными компонентами, такими как репозитории и валидаторы.
- Отделение логики от контроллеров: Контроллеры или другие компоненты, которые обрабатывают запросы, делегируют выполнение бизнес-логики сервисам, что делает их код более чистым и легким для понимания.
- Упрощение тестирования: Логику сервиса можно легко тестировать отдельно от других компонентов.
Преимущества подхода Service Layer:
- Разделение ответственности: Сервисы сосредотачиваются исключительно на бизнес-логике, освобождая контроллеры от лишней работы.
- Повторное использование: Логику, реализованную в сервисах, можно легко использовать в разных частях приложения.
- Масштабируемость: Сервисы позволяют легче расширять и модифицировать функциональность приложения.
Так же, мои замечания уже по-моему коду.
Про срезы:
Я сразу понял, что срезы start/stop можно использовать как пагинацию через Pagination или же LimitOffsetPagination у rest_framework.
Но тогда пришлось бы писать в них кастомную логику и с этим проблем нет, проблема в том, что у меня не было времени на выполнения тестового. Точнее оно было, но совсем чуть-чуть по причину того, что мне сразу сказли, что дедлайн уже завтра.
И так как я понял, что я трачу время на размышления как идеально сделать пагинацию, я решил упростить этот момент, а то я бы не успел ничего. Но в реальном кейсе я бы рассматривал наследование от Pagination.
Только его нужно было бы переписать так как пагинация и срезы это не совсем одно и тоже и на этот счет я как раз много думал.
Про фильтры:
Фильтры можно сделать используя filterset_class у вью и написать свой кастомный фильтр наследуясь от FilterSet.
Причина почему я сделал две функции фильтров все та же, из-за очень ограниченного времени. И да, эти функции имееют в себе много общего и разумно было бы подумать как можно было бы их объеленить.
Я считаю что как раз свой FilterSet помог бы это сделать. Но тогда нужно переписать лоигику в сервис классах. А лучше конечно написать свою фильтр ноду, но это время.
Про валидаторы:
Два валидатора с идентичными методами. Я специально не стал создавать базовый клас от которго можно наследоваться так как решил, что все таки два разныз валидатора работают с разными моделями и для маштабируемости оставил каждый отдельно.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment