contacts's Introduction
contacts's People
contacts's Issues
Add contact add/edit screen
Ревью кода
-
Дизайн слишком отличается в некоторых моментах. Желательно чтобы дизайн тех частей, что есть на макете соответствовал и в приложении. Особенно не забывай обращать внимания на бросающиеся мелочи (цвет текста, отступы линий и т.п.)
-
Заставлять пользователя заполнять все поля не очень юзер-френдли подход. Вспомни, что ты сам обычно заполняешь, чтобы добавить новый контакт в телефон?)) Думаю основными для заполнения должны быть только имя либо фамилия и номер телефона. Остальное можно опциональным сделать.
-
Внимательнее смотри на элементы пользовательского интерфейса: если нажать на стрелочку в выборе рингтона, то окна с выбором не появился. Появляется оно только с нажатием на само поле
-
У тебя в di находится
AppModule
. По сути вся di и рассчитана на то, что оно занимается неким объединением всего кода, поэтому предлагаю переименовать директорию вapp
и перенести тудаContactsApplication
-
Раз объекты из constants используются только в
AppModule
, то почему бы не перенести их туда и не сделать приватными? Также константные значения пишутся капсом и через нижнее подчеркивание -
AppModule
предлагаю разбить на 2 отдельных объекта. В одном будут простоfactory
иsingle
элементы, во втором же будут толькоviewModel
. Такое разделение немного сократит код в одном большом файле и чисто логически разделяет использование -
EnumSafeValueOf
лучшеextentions
называть так, для какого класса они используются, чтобы в него можно было добавить какие-либо еще функции, а не городить еще отдельный файл. Предлагаю переименовать вEnumExtentions
. С другими экстеншенами предлагаю сделать аналогично -
Fragment.makeDialog
для диалоговых окон нужно использовать ихFragment
версию. Делается это для того, чтобы при пересоздании экрана все осталось также, как и было. В данном подходе dialog просто исчезнет. Предлагаю логику создания диалога перенести в отдельный классFragmentDialog
. -
Слишком большие функции
onViewCreated
иonCreateView
. В большинстве случаев можно конечно поставить пустые строчки, чтобы было удобнее читать, но лучше все же вынести в отдельные функции, чтобы точки входа были максимально короткими -
Почаще разделяй пустой строчкой некоторые блоки кода. Часто вижу в коде как код идет сплошняком, хотя чисто логически начался другой блок логики
-
Давай придерживаться одного формата структуры класса: сначала идет компаньон (при наличии), потом публичные переменные, приватные, необходимые override функции (как для activity и фрагментов например точки входа), публичные функции, приватные и override функции от реализуемых интерфейсов. Пройдись по всему проекту и поправь, где это не так
-
ContactsListItemDecorator
встречаются магические константы (8f например), а также функцияonDraw
достаточно большая. По идее приonDraw
может понадобится что-то еще делать, поэтому предлагаю текущий код вынести в отдельную функцию внутри класса. -
ContactsListFragment
:(activity as MainActivity).supportActionBar?.setDisplayHomeAsUpEnabled(true)
Такой код немного рушит концепцию фрагментов. По сути у тебя этот фрагмент жестко закреплен за твоей MainActivity
. Если ты попытаешься использовать этот фрагмент в другой активности, то приложение просто упадет. Реализация фрагментов в идеале вообще не должна знать к какой именно активности она прикреплена. Чтобы избежать такого можно, к примеру, приводить не к MainActivity
а к типу активности, которая используется в проекте (в твоем случае это AppCompatActivity
). Прям идеальное решение это сделать интерфейс, который должна реализовывать активности и вызывать его в этом месте.
-
ContactsListAdapter
Лучше объектDiffUtil
поместить в компаньон адаптера -
GlideImageLoader
Лучше сделать объектом, так как каждый раз создавать его это лишние трудозатраты, а загрузка картинки может понадобиться часто. Также говоря о структуре проекта: не очень хорошо, когда классы просто так валяются в общей директории (исключением может быть разве чтоMainActivity
, но и то на больших проектах ее помещают в определенный флоу). Предлагаю создать отдельную директориюcommon
и поместитьGlideImageLoader
туда. Также директориюextentions
тоже положить в нее, так как она относится только кpresentation
слою. Учитывая замечание выше у тебя получится 2 директорииapp
иpresentation
, где в первой все для DI, а во второй уже presentation логика твоего проекта -
ContactsListViewModel
Достаточно прикольно, что ты реализовал смену темы приложения, но сейчас она доставит тебе немного проблем) Дело в том, что в этом классе ты напрямую обращаешься к элементу изdata
слоя. По сути это не запрещается согласно чистой архитектуре, но в твоем подходе немного смешивается логика: тема приложения это, по сути, полностьюpresentation
логика. Но вот добавление смены темы с ее сохранением уже рассматривается как отдельный значимый флоу. В таком случае необходимо описать и бизнес логику твоего флоу. Поэтому лучше все же сделать отдельный юзкейс на получение и сохранение темы приложения. Также такой подход избавит тебя от лишнегоlateinit
во вью модели -
ContactsListViewModel#obtainEvent
Тут ты также напрямую обновляешь данные обращаясь кdata
слою. Тебе нужно вpresentation
подписаться на флоу изdomain
слоя, и он уже берет данные изdata
слоя. При обновлении той же сортировки ты можешь либо подписаться на другой флоу (что не очень правильно), либо просто при вызове функции менять параметр по которому сортируешь. Опять же: это значимый кейс для пользователя и он должен проходить через доменную логику приложения. -
Чтобы не городить большую колбасень из UseCase<Flow<Result...дальшеленьписать>> достаточно сделать для каждого юзкейса свой интерфейс и чтобы он уже наследовался от колбасени выше. Таким образом ты только один раз напишешь её, а работать будешь уже с производным интерфейсом.
-
ContactsListFragment#contactsListAdapter
тут ты в фрагменте явно задаешь куда нужно будет перенаправиться, так делать неправильно. Перенаправлением на другие экраны должен отвечать родительский элемент, в твоем случае это активность. Для этого просто во фрагменте вызываешь колбэк лисенера, который должна реализовывать активность и уже в активности делаешь переходы куда тебе нужно. Иначе у тебя фрагмент становится непереиспользуемым.
UPD: посмотрел по другим фрагментам -- такая же ситуация, нужно исправить -
ContactDetailsFragment
хранишь значимую информацию внутри фрагментаcontact
, хотя всю информацию нужно хранить во вью модели. Так ты гарантируешь, что ничего не потеряется при пересоздании фрагментов. -
Создание интентов для получения номеров, открытия браузера и т.п. лучше вынести в отдельный объект утилиты
IntentUtil
, и вызывать уже метод этого объекта -
Вся логика должна находится внутри вью модели. Того же касается и валидации. Поэтому валидацию полей нужно перенести из фрагмента во вью модель
-
ViewMode
Название класса очень путает (поначалу подумал что просто ViewModel не дописал)), лучше переименовать вViewState
илиFragmentState
-
Обычно не используются репозитории. Чаще всего юзают
DataSource
(у репозитория немного другая логическая задача). И если делать уже какDataSource
, то в реализации не должно быть никакиеtry catch
(только в очень редких случаях, когда это необходимо на проекте, но это не твой случай). Вся обработка должна быть вdomain
слое. -
У тебя в юзкейсах повторяется один и тот же код. Его можно добавить как базовый в
run
функцию твоего юзкейса, просто чтобы передавался блок кода, а сам перехват уже был зашит в run. И если нужно будет вызвать функцию без этого шаблона, можно создать какую-нибудь функциюinvoke
-
data
иdomain
слои: лучше расположение файлов сделать такое же, как и вpresentation
слое. То есть нужно сделать расположение по флоу, а не поusecase
,model
и т.п. Это пока что приложение достаточно маленькое, если оно станет больше, то ты просто не сможешь найти нужный юзкейс в директории с 150+ файлами юзкейсов)
В целом достаточно хорошо всё реализовано и фича со сменой темы хороша) А айтем декоратор для букв вообще пушка
Add contact details screen
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.