Skip to content

дз 23 спринт.#16

Open
FatefulCost wants to merge 2 commits intomasterfrom
dev
Open

дз 23 спринт.#16
FatefulCost wants to merge 2 commits intomasterfrom
dev

Conversation

@FatefulCost
Copy link
Owner

No description provided.

R.id.createPlaylistFragment,
R.id.playlistFragment -> {
// Скрываем BottomNavigationView
bottomNavigationView.visibility = View.GONE

Choose a reason for hiding this comment

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

У view есть удобный экстеншен: View.isVisible = true/false, которая заменяет привычное нам View.visibility = View.VISIBLE/View.GONE


override suspend fun removeTrackFromPlaylist(playlistId: Long, trackId: Long) {
Log.d(TAG, "removeTrackFromPlaylist: playlistId=$playlistId, trackId=$trackId")
val playlist = getPlaylistById(playlistId)

Choose a reason for hiding this comment

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

можно сделать ранний выход, тогда не потребуется оборачивать код ниже в if-блок:
val playlist = getPlaylistById(playlistId) ?: return

Comment on lines +53 to +55
Log.d("PlaylistInteractor", "========== DELETE PLAYLIST CALLED ==========")
Log.d("PlaylistInteractor", "deletePlaylist called with id: $playlistId")
Log.d("PlaylistInteractor", "Thread: ${Thread.currentThread().name}")

Choose a reason for hiding this comment

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

при многострочном логировании можно применять строку с форматированием:

Log.d("PlaylistInteractor", """что-то на несколько строк""".trimIndent() или .trimMargin())

Comment on lines +42 to +44
private var isEditMode = false
private var editingPlaylist: Playlist? = null
private var originalCoverPath: String? = null

Choose a reason for hiding this comment

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

лучше разруливать и хранить все на уровне вьюмодели, а для редактирования можно хранить только id плейлиста

фрагменты/активити должны отвечать только за отрисовку

Comment on lines +21 to +22
private val _showEmptyShareMessage = MutableLiveData<Boolean>()
val showEmptyShareMessage: LiveData<Boolean> = _showEmptyShareMessage

Choose a reason for hiding this comment

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

для одиночных операция вроде показа тостов/снекбаров или навигации лучше посмотреть в сторону SingleLiveData - это обертка над liveData которая делает их максимально одиночными (события не будут повторяться при смене конфигурации)

appendLine(playlist.description)
}
val tracksCountText = when {
tracks.size % 10 == 1 && tracks.size % 100 != 11 -> "${tracks.size} трек"

Choose a reason for hiding this comment

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

Воспользуйся лучше plurals, в андроиде уже реализована логика, с помощью которой для каждой локали будет подбираться соответствующее окончание. Почитать о том, как это использовать, можно в официальной документации - https://developer.android.com/guide/topics/resources/string-resource#Plurals

Comment on lines +16 to +29
private val _playlist = MutableLiveData<Playlist?>()
val playlist: LiveData<Playlist?> = _playlist

private val _tracks = MutableLiveData<List<Track>>()
val tracks: LiveData<List<Track>> = _tracks

private val _totalDuration = MutableLiveData<String>()
val totalDuration: LiveData<String> = _totalDuration

private val _shareText = MutableLiveData<String?>()
val shareText: LiveData<String?> = _shareText

private val _showEmptyShareMessage = MutableLiveData<Boolean>()
val showEmptyShareMessage: LiveData<Boolean> = _showEmptyShareMessage

Choose a reason for hiding this comment

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

Очень много лайвдат - попробуй разделить на две:

  1. отвечает за стейт экрана (список, заголовок и прочая информация)
  2. SingleLiveData (комменты выше) - обрабатывать единичные ивенты (шаринг текста/показ тоста и тд)

}
}

private fun getTracksCountText(count: Int): String {

Choose a reason for hiding this comment

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

это второй метод с тем же поведением, лучше обратись к plurals

Copy link

@sukhoikms27 sukhoikms27 left a comment

Choose a reason for hiding this comment

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

Рекомендации к первой итерации:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants