todo-list's People
todo-list's Issues
[Рефакторинг и обратное проектирование] Задание 3
До рефакторинга система имела следующий вид, изображенный на схеме:
Данной системе был произведен рефакторинг с учетом задания #1 и еще некоторых размышлений автора.
В итоге была реализованная следующая система, изображенная на схеме:
Что было сделано:
- Вся логика была вынесена из компонентов. Компонент TodoListPageComponent знает как получать список задач и передает его компонентам для отображения. Остальные компоненты только отображают данные и могут хранить некоторое внутренне состояние компонента.
- Компоненты используют один интерфейс модели Todo.
- Выделен отдельный сервис для работы с диалоговыми окнами.
- Добавлен Store это библиотека ngrx, реализующая паттерн Flux. Таким образом имеем единый источник хранения данных в системе и все действия проходят через него.
- Выделены FirebaseTodoBackendService и TodoService. FirebaseTodoBackendService отвечает за работу с Backend. TodoService предоставляет интерфейс для работы приложению и содержит логику обработки конфликтов.
После рефакторинга была проверена работоспособность системы используя тесты написанные в рамках задания #2
Код с изменениям можно найти в PR #4
[Рефакторинг и обратное проектирование] Задание 2
Перед рефакторингом необходимо покрыть код тестами. Недочеты, выбранные в Задании 1 невозможно покрыть юнит-тестами, так как или для случая нельзя написать тест или код будет перенесен/удален/изменен и тесты придется писать снова. Поэтому было решено написать e2e тесты.
Тестовый файл
Вспомогательный файл страницы
[Рефакторинг и обратное проектирование] Задание 1
-
Ленивый класс и класс данных. Класс не делает ничего. В текущей реализации можно заменить на интерфейс или выделить функции и добавить в класс.
todo-list/src/app/todo-list/todo.model.ts
Lines 1 to 10 in c0b6b01
-
Каждый элемент имеет текущее состояние (Добавлен, Изменен или Удален). Из-за этого код содержит множество switch условий, в разных классах, что будет затруднять дальнейшее сопровождение. Если потребуется вносить изменения, то придется найти все switch условия и изменить каждый.
todo-list/src/app/todo-list/todo-list.component.ts
Lines 104 to 116 in c0b6b01
todo-list/src/app/todo-list/todo-item/todo-item.component.ts
Lines 113 to 121 in c0b6b01
Ангуляр это MVVM фреймворк, основными блоками которого являются "компоненты" и "сервисы".
- Компонент это класс, связанный с View, который может передавать данные во View и реагировать на события View. Он является View Model из MVVM. Класс должен получать данные и отображать их. В идеале без какой либо логики. Компоненты делятся на smart и dumb, которые содержат логику (знают как получить данные, создать что-то и тд) и которые не содержат логику, а только отображают данные, соответственно.
- Сервис это класс с Dependency Injection. Сервисы должны содержать всю бизнес логику и обратку в приложении.
- Компонент todo-item.component.ts, который отвечает за отображение на экране одного элемента списка, содержит больше количество логики внутри себя. Это затрудняет понимание класса и создает путаницу. Из очевидного:
- Необходимо вынести код, который отвечает за открытие диалоговых окон и их вид.
- Необходим вынести код, который отвечает за обработку конфликтов при параллельном изменении одних и тех-же данных в разных сессиях.
-
Странное решение сгруппировать все типы действий, которые можно совершить над элементов списка и передать их через один "Output" (выход) в родительский класс, вместо того чтобы создать на каждое действие свой выход.
todo-list/src/app/todo-list/todo-item/todo-item.component.ts
Lines 70 to 84 in c0b6b01
В итоге из-за этого имеем лишний switch.
todo-list/src/app/todo-list/todo-list.component.ts
Lines 51 to 66 in c0b6b01
Если не вдаваться в код, то так же странно, что элемент списка имеет действие "создать". По идее, над элементом можно совершить 2 действия или обновить или удалить. В идеале необходимо избавиться от этого "лишнего" действия. -
Компонент todo-list.component.ts сам создает новые элементы списка. Если у нас будут другие компоненты, где можно создать новый элемент, то потом придется менять код создания во всех местах. Создание должно проходить в единственном месте. Необходимо его вынести в сервис.
todo-list/src/app/todo-list/todo-list.component.ts
Lines 44 to 48 in c0b6b01
-
Компонент todo-list.component.ts оперирует двумя интерфейсами для элементов. Где-то используется Todo, а где-то TodoMetadata. Это вводит путаницу.
todo-list/src/app/todo-list/todo-list.component.ts
Lines 68 to 78 in c0b6b01
Тем более класс TodoMetadata зависит от Todo. Можно было бы изначально использовать 2 класса. Первый, как модель хранится на сервере, второй, серверная модель + дополнительные данные, типо id и прочее, которые уже используются приложением, без лишней вложенности.
todo-list/src/app/todo-list/todo.model.ts
Lines 1 to 16 in c0b6b01
-
Временное поле.
Данное поле заполняется и используется только когда существует конфликт при одновременном изменении одного и того же элемента. Данное поле должно удалиться при выделении логики и конфликтах в самостоятельный класс.
-
Компонент todo-item.component.ts, имеет функциональность изменения заголовка элемента. В текущей реализации мы сами проверяем целостность данных, введенных пользователем. Данный функционал можно реализовать через Reactive Form и добавить валидацию предоставляемую формами, и сделать более гибкую настройку. К тому же при изменении требований по валидации, очень легко и можно добавлять свои собственные функции для валидации ввода.
-
Компонент todo-item.component.ts содержит много логики, которой он содержать не должен. Данный компонент должен лишь отображать список Todo. Компонент в свою очередь знает как получить данные, как обрабатывать эти данные в зависимости от типа с сервера и на основании решения добавляет или удаляет в список, создает новые списки Todo.
-
Отсутствует обработки ошибок.
Сервис может выбросить ошибку, при неверных данных, а так как используется класс без геттеров и сеттверов, то поле может быть не валидным или быть удалено.
todo-list/src/app/todo-list/todo-list.service.ts
Lines 18 to 20 in c0b6b01
todo-list/src/app/todo-list/todo-list.service.ts
Lines 35 to 37 in c0b6b01
Но, в коде который вызывает эти функции отсутствует перехватчик ошибок и код может поломаться.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.