Created
July 20, 2024 13:20
-
-
Save defenitionofreal/e5428316fef257e5a54eb62e64a48860 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Зачада 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