defenitionofreal · July 20, 2024 13:20
diff --git a/gistfile1.txt b/gistfile1.txt
 Зачада 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 помог бы это сделать. Но тогда нужно переписать лоигику в сервис классах. А лучше конечно написать свою фильтр ноду, но это время.

 Про валидаторы:
 Два валидатора с идентичными методами. Я специально не стал создавать базовый клас от которго можно наследоваться так как решил, что все таки два разныз валидатора работают с разными моделями и для маштабируемости оставил каждый отдельно.
	Зачада 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 помог бы это сделать. Но тогда нужно переписать лоигику в сервис классах. А лучше конечно написать свою фильтр ноду, но это время.

	Про валидаторы:
	Два валидатора с идентичными методами. Я специально не стал создавать базовый клас от которго можно наследоваться так как решил, что все таки два разныз валидатора работают с разными моделями и для маштабируемости оставил каждый отдельно.