Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Homework 6 #4

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Homework 6 #4

wants to merge 4 commits into from

Conversation

mimakopanja
Copy link
Owner

Урок 6. - Room и хранение данных

Copy link

@PeterBukhal-Geekbrains PeterBukhal-Geekbrains left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть замечания оставил комментарии в github, в целом хорошо.


class MainActivity : MvpAppCompatActivity(), MainView {

private lateinit var viewBinding: ActivityMainBinding

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Использование lateinit является очень плохой практикой, и допускается только для внедрения зависимостей в компоненты Android (Activity, Fragment и тд) с использованием Dagger 2. Для всех (любых) случаев это абсолютно неприемлемая практика от которой нужно отказаться.

btnCounter2.setOnClickListener(listener)
btnCounter3.setOnClickListener(listener)
}
viewBinding = ActivityMainBinding.inflate(layoutInflater)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Удобно использовать https://github.com/kirich1409/ViewBindingPropertyDelegate. См. последний пример с разбором ДЗ урока 2.


interface IImageCache {
fun saveImage(url: String, bytes: ByteArray): Completable
fun getCachedImage(url: String): Maybe<ByteArray?>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RxJava не работает null типами. если в цепочке будет null ссылка то будет фатал.

it.userId
)
}
}.subscribeOn(Schedulers.io())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Планировщик потоков нужно передавать в конструкторе в виде зависимости (помним про Dependency Injection и Dependency Inversion), иначе его не заменить из вне.

)
}
database.repositoryDao.insert(roomRepos)
}.subscribeOn(Schedulers.io())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Планировщик потоков нужно передавать в конструкторе в виде зависимости (помним про Dependency Injection и Dependency Inversion), иначе его не заменить из вне.

): IGithubUsersCache {

override fun getUsers() = Single.fromCallable {
database.userDao.getAll().map { roomUser ->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хорошей практикой написания реактивных цепочек является написание их в каждой новой строке, это улучшает читаемость.

import moxy.MvpPresenter


class MainPresenter(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Форматирование кода конечно страдает. Такой код сложнее воспринимать. Используй Ctrl+Alt+L.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants