Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

내가 업로드한 노래 전체 보기 #225

Merged
merged 10 commits into from
Nov 29, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.ohdodok.catchytape.feature.mypage

import android.os.Bundle
import android.view.View
import androidx.fragment.app.viewModels
import com.ohdodok.catchytape.core.ui.BaseFragment
import com.ohdodok.catchytape.core.ui.MusicAdapter
import com.ohdodok.catchytape.core.ui.Orientation
import com.ohdodok.catchytape.core.ui.toMessageId
import com.ohdodok.catchytape.feature.mypage.databinding.FragmentMyMusicsBinding
import dagger.hilt.android.AndroidEntryPoint

@AndroidEntryPoint
class MyMusicsFragment : BaseFragment<FragmentMyMusicsBinding>(R.layout.fragment_my_musics) {
private val viewModel: MyMusicsViewModel by viewModels()

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
binding.viewModel = viewModel

setupRecyclerView()
observeEvents()
setupBackStack(binding.tbMyMusics)
}

private fun setupRecyclerView() {
binding.rvMyMusics.adapter = MusicAdapter(Orientation.VERTICAL)
}

private fun observeEvents() {
repeatOnStarted {
viewModel.events.collect { event ->
when (event) {
is MyMusicsEvent.ShowMessage -> {
showMessage(event.error.toMessageId())
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.ohdodok.catchytape.feature.mypage

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.ohdodok.catchytape.core.domain.model.CtErrorType
import com.ohdodok.catchytape.core.domain.model.CtException
import com.ohdodok.catchytape.core.domain.model.Music
import com.ohdodok.catchytape.core.domain.repository.MusicRepository
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.coroutines.plus
import javax.inject.Inject

data class MyMusicsUiState(
val myMusics: List<Music> = emptyList()
)

sealed interface MyMusicsEvent {
data class ShowMessage(
val error: CtErrorType
) : MyMusicsEvent
}

@HiltViewModel
class MyMusicsViewModel @Inject constructor(
private val musicRepository: MusicRepository,
) : ViewModel() {

private val _uiState = MutableStateFlow(MyMusicsUiState())
val uiState: StateFlow<MyMusicsUiState> = _uiState.asStateFlow()

private val _events = MutableSharedFlow<MyMusicsEvent>()
val events: SharedFlow<MyMusicsEvent> = _events.asSharedFlow()

private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

    private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
        val errorType =
            if (throwable is CtException) throwable.ctError
            else CtErrorType.UN_KNOWN

        viewModelScope.launch { _events.emit(PlaylistsEvent.ShowMessage(errorType)) }
    }

emit 할때만, viewModelScope 이용하면 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

            val errorType =
                if (throwable is CtException) throwable.ctError
                else CtErrorType.UN_KNOWN

errorType이 viewModelScope.launch 안에 있는데, 이걸 viewModelScope.launch 바깥으로 빼자는 의미입니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 고치긴 했는데 이유는 무엇인가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

코루틴 안에서 할 필요가 없는 일을, 굳이 코루틴 안에서 하는게 이상하다는 것 이였습니다~

viewModelScope.launch {
val errorType =
if (throwable is CtException) throwable.ctError
else CtErrorType.UN_KNOWN

_events.emit(MyMusicsEvent.ShowMessage(errorType))
Comment on lines +47 to +51
Copy link
Member

@youlalala youlalala Nov 29, 2023

Choose a reason for hiding this comment

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

읽기 편해져서 좋은 것 같네욥 👏🏻

}
}

private val viewModelScopeWithExceptionHandler = viewModelScope + exceptionHandler

init {
fetchMyMusics()
}
Comment on lines +57 to +59
Copy link
Collaborator

Choose a reason for hiding this comment

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

왜 init { } 에서 호출하는지, 알 수 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

MyPage는 onStart 시점에 호출하도록 변경했어
여기는 이 위치가 맞는 거 같은데 코멘트를 단 이유가 궁금해

Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 MyMusicsViewModel.kt 에서 호출 되는지 착각 했네요,
MyPageViewModel.kt 에서 init { fetchMyMusics() } 하는 줄 착각해서 코멘트 달았습니다~

이 위치 맞습니다~


private fun fetchMyMusics() {
musicRepository.getMyMusics()
.onEach { musics ->
_uiState.update {
it.copy(myMusics = musics)
}
}
.launchIn(viewModelScopeWithExceptionHandler)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ package com.ohdodok.catchytape.feature.mypage
import android.os.Bundle
import android.view.View
import androidx.fragment.app.viewModels
import androidx.navigation.fragment.findNavController
import com.ohdodok.catchytape.core.ui.BaseFragment
import com.ohdodok.catchytape.core.ui.MusicAdapter
import com.ohdodok.catchytape.core.ui.Orientation
import com.ohdodok.catchytape.core.ui.toMessageId
import com.ohdodok.catchytape.feature.mypage.databinding.FragmentMyPageBinding
import dagger.hilt.android.AndroidEntryPoint
Expand All @@ -18,7 +21,8 @@ class MyPageFragment : BaseFragment<FragmentMyPageBinding>(R.layout.fragment_my_
binding.viewModel = viewModel

observeEvents()
viewModel.fetchMyMusics()
setupButtons()
setupRecyclerView()
}

private fun observeEvents() {
Expand All @@ -32,4 +36,20 @@ class MyPageFragment : BaseFragment<FragmentMyPageBinding>(R.layout.fragment_my_
}
}
}

private fun setupButtons() {
binding.btnMore.setOnClickListener {
val action = MyPageFragmentDirections.actionMyPageFragmentToMyMusicsFragment()
findNavController().navigate(action)
}
}

private fun setupRecyclerView() {
binding.rvMusics.adapter = MusicAdapter(Orientation.VERTICAL)
}

override fun onStart() {
super.onStart()
viewModel.fetchMyMusics(count = 3)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,20 @@ class MyPageViewModel @Inject constructor(
val events: SharedFlow<MyPageEvent> = _events.asSharedFlow()

private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
viewModelScope.launch {
if (throwable is CtException) {
_events.emit(MyPageEvent.ShowMessage(throwable.ctError))
} else {
_events.emit(MyPageEvent.ShowMessage(CtErrorType.UN_KNOWN))
}
}
val errorType =
if (throwable is CtException) throwable.ctError
else CtErrorType.UN_KNOWN

viewModelScope.launch { _events.emit(MyPageEvent.ShowMessage(errorType)) }
}

private val viewModelScopeWithExceptionHandler = viewModelScope + exceptionHandler


fun fetchMyMusics() {
fun fetchMyMusics(count: Int) {
musicRepository.getMyMusics()
.onEach { musics ->
_uiState.update {
it.copy(myMusics = musics.take(3))
it.copy(myMusics = musics.take(count))
}
}
.launchIn(viewModelScopeWithExceptionHandler)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">

<data>

<variable
name="viewModel"
type="com.ohdodok.catchytape.feature.mypage.MyMusicsViewModel" />
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="match_parent">

<com.google.android.material.appbar.MaterialToolbar
android:id="@+id/tb_my_musics"
android:layout_width="0dp"
android:layout_height="@dimen/appbar_height"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:navigationIcon="@drawable/ic_arrow_back"
app:title="@string/my_musics" />

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/rv_my_musics"
android:layout_width="0dp"
android:layout_height="0dp"
android:clipToPadding="false"
android:paddingHorizontal="@dimen/margin_horizontal"
android:paddingTop="@dimen/large"
app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/tb_my_musics"
app:list="@{viewModel.uiState.myMusics}"
tools:listitem="@layout/item_music_vertical" />

</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
android:layout_height="match_parent">

<com.google.android.material.appbar.MaterialToolbar
android:id="@+id/mt_my_page"
android:id="@+id/tb_my_page"
android:layout_width="0dp"
android:layout_height="@dimen/appbar_height"
app:layout_constraintEnd_toEndOf="parent"
Expand All @@ -34,7 +34,7 @@
android:text="@string/share_music"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/mt_my_page" />
app:layout_constraintTop_toBottomOf="@id/tb_my_page" />

<androidx.appcompat.widget.AppCompatButton
android:id="@+id/btn_upload"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,18 @@
<fragment
android:id="@+id/my_page_fragment"
android:name="com.ohdodok.catchytape.feature.mypage.MyPageFragment"
android:label="mypage"
android:label="my page"
Copy link
Collaborator

Choose a reason for hiding this comment

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

마이너 한데,
my_page 는 어떠신가요? (my_musics 도 마찬가지 입니다~)

Copy link
Member Author

Choose a reason for hiding this comment

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

label은 사용하는 용도가 아니라 Preview에서 읽는 용도인데, my_page보다 my page가 가독성 좋지 않을까?

Copy link
Collaborator

Choose a reason for hiding this comment

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

가독성은 비슷할것 같네요, snack_case를 예외 없이 사용하자~ 라는 의미였습니다~

tools:layout="@layout/fragment_my_page">

<action
android:id="@+id/action_my_page_fragment_to_my_musics_fragment"
app:destination="@id/my_musics_fragment" />
</fragment>

<fragment
android:id="@+id/my_musics_fragment"
android:name="com.ohdodok.catchytape.feature.mypage.MyMusicsFragment"
android:label="my musics"
tools:layout="@layout/fragment_my_musics" />

</navigation>