kane2033 / ContactsKODE

0 stars 0 forks source link

CodeReview #1

Open VadimMoschenkov opened 3 years ago

VadimMoschenkov commented 3 years ago

UiState и FailureInfo

Не совсем понятно зачем UiState в домене, для домена UI быть не должно вообще как информации о том как он работает. В классе FailureInfo есть поле с текстом кнопки а это упоминание конкретного UI такого нужно избегать в доменном слое

ContactsDataSource#addContactsList

этот метод не используется

NoInternet, ServerFailure, Timeout, MissingContentFailure и т.д.

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

ContactsDataSourceImpl

в конструктор передается context, а далее через статический метод получаем БД и дао, в конструкор можно было сразу передать ContactDao и использовать тут DI

ContactEditViewModel

ContactEditViewModel#mode - поле публичное, и это не безопасно, нужно использовать приватный сетер

kane2033 commented 3 years ago

UiState и FailureInfo

Перенёс классы в presentation слой, потому что они используются исключительно там. [4e265f1]

ContactsDataSource#addContactsList

Убрал неиспользуемый код, с одним исключением (ниже). [a69d098]

NoInternet, ServerFailure, Timeout, MissingContentFailure и т.д.

Не убрал неиспользуемые Failure, а оставил комментарий о том, что данные Failure понадобятся, если будет необходимость в использовании сети (пр. синхронизация контактов с сервером). [a69d098]

ContactsDataSourceImpl

Создал статический метод у ContactsDatabase#getContactDao, который вызывается в DI. ContactsDataSourceImpl в конструктор теперь получает ContactDao. [228d6a8]

ContactEditViewModel

Не очень понял по поводу "приватного сетера", потому что mode это val, которая инициализируется в конструкторе вьюмодели. Так или иначе, сделал mode приватным полем и создал метод ContactEditViewModel#isContactNew, возвращающий true, если на экране создается новый контакт, а не редактируется существующий. [4def8b3]

Также исправил ошибку, при которой уже сохраненный рингтон пытался сохраниться еще раз. Для этого, при сохранении внешнего файла во внутреннюю память, проверяется существование сохраняемого файла. [9c354b1a640168196d2cd3c3dc6baf97080e801d]