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.

Sign up for GitHub

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

Jump to bottom

[AN] 스터디 개설 뷰 리펙터링 #618

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

[AN] 스터디 개설 뷰 리펙터링 #618

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
43ed43d
refactor: DataBinding -> ViewBinding
inseonyun Nov 21, 2023
bdcf283
refactor: flow collect 확장함수 사용
inseonyun Nov 21, 2023
602a632
refactor: FirstFragment DataBinding -> ViewBinding
inseonyun Nov 21, 2023
23511e3
refactor: 스터디개설 뷰 바인딩 어댑터 제거
inseonyun Nov 21, 2023
50f7950
refactor: SecondFragment DataBinding -> ViewBinding
inseonyun Nov 21, 2023
7f3ba5d
refactor: flow collect 확장함수 사용
inseonyun Nov 21, 2023
26d6b1f
refactor: 스터디 개설 이벤트 작성
inseonyun Nov 21, 2023
a9143f9
refactor: BindingViewBottomSheetFragment 작성
inseonyun Nov 21, 2023
4c712f7
refactor: CycleBottomSheetFragment DataBinding -> ViewBinding
inseonyun Nov 21, 2023
2c9120e
refactor: PeopleCountBottomSheetFragment DataBinding -> ViewBinding
inseonyun Nov 21, 2023
e9b0b52
refactor: StudyDateBottomSheetFragment DataBinding -> ViewBinding
inseonyun Nov 21, 2023
d109ec5
refactor: collect 함수명 수정
inseonyun Nov 21, 2023
e2221ad
refactor: Combine Flow -> StateFlow 변경
inseonyun Nov 21, 2023
0fa5b4f
refactor: CreateStudyEvent 작성
inseonyun Nov 21, 2023
5964bea
refactor: CreateStudyState 수정
inseonyun Nov 21, 2023
e6e10bf
refactor: 함수 분리
inseonyun Nov 21, 2023
345c394
refactor: CreateStudy State Idle 상태 추가
inseonyun Nov 21, 2023
771d091
refactor: ScrollView 작성
inseonyun Nov 21, 2023
eb5c465
refactor: StateFlow Emit -> setValue
inseonyun Nov 23, 2023
773d28c
refactor: combine 불필요한 return 제거
inseonyun Nov 23, 2023
77cc089
refactor: 매직넘버 상수화
inseonyun Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions ...p/src/main/java/com/created/team201/presentation/common/BindingViewBottomSheetFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.created.team201.presentation.common

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.viewbinding.ViewBinding
import com.created.team201.R
import com.google.android.material.bottomsheet.BottomSheetDialogFragment

abstract class BindingViewBottomSheetFragment<VB : ViewBinding>(
private val bindingInflater: (LayoutInflater) -> VB
) : BottomSheetDialogFragment() {
private var _binding: VB? = null
val binding get() = _binding ?: error(R.string.error_baseFragment)

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
): View {
_binding = bindingInflater(inflater)
return binding.root
}

override fun onDestroyView() {
super.onDestroyView()
_binding = null
}
}
118 changes: 58 additions & 60 deletions ...oid/app/src/main/java/com/created/team201/presentation/createStudy/CreateStudyActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,40 @@ import androidx.activity.viewModels
import androidx.annotation.StringRes
import androidx.fragment.app.Fragment
import androidx.fragment.app.commit
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.created.team201.R
import com.created.team201.databinding.ActivityCreateStudyBinding
import com.created.team201.presentation.common.BindingActivity
import com.created.team201.presentation.createStudy.model.CreateStudyUiState.Fail
import com.created.team201.presentation.createStudy.model.CreateStudyUiState.Idle
import com.created.team201.presentation.createStudy.model.CreateStudyUiState.Success
import com.created.team201.presentation.common.BindingViewActivity
import com.created.team201.presentation.createStudy.CreateStudyViewModel.CreateStudyState.Failure
import com.created.team201.presentation.createStudy.CreateStudyViewModel.CreateStudyState.Idle
import com.created.team201.presentation.createStudy.CreateStudyViewModel.CreateStudyState.Loading
import com.created.team201.presentation.createStudy.CreateStudyViewModel.CreateStudyState.Success
import com.created.team201.presentation.createStudy.CreateStudyViewModel.Event.CreateStudyFailure
import com.created.team201.presentation.createStudy.CreateStudyViewModel.Event.CreateStudySuccess
import com.created.team201.presentation.createStudy.CreateStudyViewModel.Event.NavigateToBefore
import com.created.team201.presentation.createStudy.CreateStudyViewModel.Event.NavigateToNext
import com.created.team201.presentation.createStudy.model.FragmentState
import com.created.team201.presentation.createStudy.model.FragmentState.FirstFragment
import com.created.team201.presentation.createStudy.model.FragmentState.SecondFragment
import com.created.team201.presentation.createStudy.model.FragmentType
import com.created.team201.presentation.createStudy.model.FragmentType.FIRST
import com.created.team201.presentation.createStudy.model.FragmentType.SECOND
import com.created.team201.presentation.studyDetail.StudyDetailActivity
import com.created.team201.util.collectLatestOnStarted
import com.created.team201.util.collectOnStarted
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.launch

@AndroidEntryPoint
class CreateStudyActivity :
BindingActivity<ActivityCreateStudyBinding>(R.layout.activity_create_study) {
BindingViewActivity<ActivityCreateStudyBinding>(ActivityCreateStudyBinding::inflate) {
private val createStudyViewModel: CreateStudyViewModel by viewModels()

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

initBinding()
initActionBar()
setupCollectCreateStudyState()
setupCollectCreateStudyUiState()
showFragment(createStudyViewModel.fragmentState.value.type)
collectCreateStudyState()
collectCreateStudyEvent()
}

override fun dispatchTouchEvent(ev: MotionEvent): Boolean {
Expand All @@ -51,8 +54,9 @@ class CreateStudyActivity :
return super.dispatchTouchEvent(ev)
}

private fun initBinding() {
binding.lifecycleOwner = this
override fun onOptionsItemSelected(item: MenuItem): Boolean {
createStudyViewModel.navigateToBefore()
return true
}

private fun initActionBar() {
Expand All @@ -62,67 +66,61 @@ class CreateStudyActivity :
supportActionBar?.setHomeAsUpIndicator(R.drawable.ic_back)
}

override fun onOptionsItemSelected(item: MenuItem): Boolean {
return when (createStudyViewModel.fragmentState.value) {
is FirstFragment -> {
finish()
true
}

is SecondFragment -> {
createStudyViewModel.navigateToBefore()
true
}
}
}

private fun setProgressIndicator(type: FragmentType) {
binding.lpiCreateStudyProgress.progress = when (type) {
FIRST -> PROGRESS_FIRST
SECOND -> PROGRESS_SECOND
}
}

private fun setupCollectCreateStudyUiState() {
lifecycleScope.launch {
createStudyViewModel.createStudyUiState.collectLatest { createStudyUiState ->
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
when (createStudyUiState) {
is Success -> {
showToast(R.string.create_study_toast_create_study_success)
startActivity(
StudyDetailActivity.getIntent(
this@CreateStudyActivity,
createStudyUiState.studyId
),
)

finish()
}

is Fail -> {
showToast(R.string.create_study_toast_create_study_fail)
finish()
}

is Idle -> throw IllegalArgumentException()
}
private fun collectCreateStudyState() {
createStudyViewModel.createStudyState
.collectLatestOnStarted(this) { createStudyState ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

왜 collectLatest를 사용하셨나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

State에 Loading과 Idle을 넣기 전에 중복 시도 시 이전 통신 값은 무시하고 마지막에 요청한거만 하도록 하려고 이렇게 했었슴다
지금은 collect든 collectLatest든 둘 다 동일한 동작을 수행할 거 같네용

when (createStudyState) {
is Success -> navigateToStudyDetail(createStudyState.studyId)
Loading -> Unit
Idle -> Unit
Failure -> finish()
}
}
}

private fun navigateToStudyDetail(studyId: Long) {
startActivity(
StudyDetailActivity.getIntent(
this@CreateStudyActivity,
studyId
),
)
finish()
}

private fun collectCreateStudyEvent() {
createStudyViewModel.createStudyEvent.collectOnStarted(this) { event ->
when (event) {
CreateStudySuccess -> showToast(R.string.create_study_toast_create_study_success)
CreateStudyFailure -> showToast(R.string.create_study_toast_create_study_fail)
is NavigateToBefore -> navigateToBefore(event.fragmentState)
is NavigateToNext -> navigateToNext(event.fragmentState)
}
}
}

private fun showToast(@StringRes messageRes: Int) {
Toast.makeText(this, getString(messageRes), Toast.LENGTH_SHORT).show()
}

private fun setupCollectCreateStudyState() {
lifecycleScope.launch {
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
createStudyViewModel.fragmentState.collect { fragmentState ->
showFragment(fragmentState.type)
}
}
private fun navigateToBefore(fragmentState: FragmentState) {
when (fragmentState) {
FirstFragment -> finish()
SecondFragment -> showFragment(FirstFragment.type)
}
}

private fun navigateToNext(fragmentState: FragmentState) {
when (fragmentState) {
FirstFragment -> showFragment(SecondFragment.type)
SecondFragment -> Unit
}
}

Expand Down
72 changes: 47 additions & 25 deletions ...id/app/src/main/java/com/created/team201/presentation/createStudy/CreateStudyViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.created.domain.model.CreateStudy
import com.created.team201.data.repository.CreateStudyRepository
import com.created.team201.presentation.createStudy.model.CreateStudyUiState
import com.created.team201.presentation.createStudy.model.FragmentState
import com.created.team201.presentation.createStudy.model.FragmentState.FirstFragment
import com.created.team201.presentation.createStudy.model.FragmentState.SecondFragment
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import javax.inject.Inject

Expand Down Expand Up @@ -44,20 +44,22 @@ class CreateStudyViewModel @Inject constructor(
MutableStateFlow(DEFAULT_STRING_VALUE)
val studyIntroduction: StateFlow<String> get() = _studyIntroduction.asStateFlow()

val isEnableFirstCreateStudyNext: Flow<Boolean> =
val isEnableFirstCreateStudyNext: StateFlow<Boolean> =
combine(peopleCount, studyDate, cycle) { peopleCount, studyDate, cycle ->
return@combine peopleCount != DEFAULT_INT_VALUE && studyDate != DEFAULT_INT_VALUE && cycle != DEFAULT_INT_VALUE
}
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false)

val isEnableSecondCreateStudyNext: Flow<Boolean> =
val isEnableSecondCreateStudyNext: StateFlow<Boolean> =
combine(studyName, studyIntroduction) { studyName, studyIntroduction ->
return@combine studyName.isNotBlankAndEmpty() && studyIntroduction.isNotBlankAndEmpty()
}
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 combine 블록 내부에
return@combine 구문이 필요한가요??
블럭 내부에 가장 마지막 줄이 return의 역할을 하지 않나요?
제가 놓치는 점이 있다면 꼭 말씀주세요! 궁금합니다.

'블록의 마지막 식이 블록의 결과'라는 규칙은 블록이 값을 만들어내야 하는 경우 항상 성립한다. - Kotlin in action


private val _createStudyUiState: MutableSharedFlow<CreateStudyUiState> = MutableSharedFlow()
val createStudyUiState: SharedFlow<CreateStudyUiState> get() = _createStudyUiState.asSharedFlow()
private val _createStudyState: MutableStateFlow<CreateStudyState> =
MutableStateFlow(CreateStudyState.Idle)
val createStudyState: StateFlow<CreateStudyState> get() = _createStudyState.asStateFlow()

private var isOpenStudy: Boolean = false
private val _createStudyEvent: MutableSharedFlow<Event> = MutableSharedFlow()
val createStudyEvent: SharedFlow<Event> get() = _createStudyEvent.asSharedFlow()

fun setPeopleCount(peopleCount: Int) {
_peopleCount.value = peopleCount
Expand All @@ -80,28 +82,29 @@ class CreateStudyViewModel @Inject constructor(
}

fun navigateToNext() {
when (_fragmentState.value) {
is FirstFragment -> {
_fragmentState.value = SecondFragment
}
viewModelScope.launch {
when (_fragmentState.value) {
is FirstFragment -> {
_createStudyEvent.emit(Event.NavigateToNext(fragmentState.value))
_fragmentState.value = SecondFragment
}

else -> Unit
else -> Unit
}
}
}

fun navigateToBefore() {
when (_fragmentState.value) {
is SecondFragment -> {
_fragmentState.value = FirstFragment
}

else -> Unit
viewModelScope.launch {
_createStudyEvent.emit(Event.NavigateToBefore(fragmentState.value))
_fragmentState.value = FirstFragment
}
}

fun createStudy() {
if (isOpenStudy) return
isOpenStudy = true
if (createStudyState.value == CreateStudyState.Loading)
return
_createStudyState.value = CreateStudyState.Loading
viewModelScope.launch {
val study = CreateStudy(
name = studyName.value.trim(),
Expand All @@ -112,15 +115,34 @@ class CreateStudyViewModel @Inject constructor(
)
createStudyRepository.createStudy(study)
.onSuccess { studyId ->
_createStudyUiState.emit(CreateStudyUiState.Success(studyId))
_createStudyEvent.emit(Event.CreateStudySuccess)
_createStudyState.emit(CreateStudyState.Success(studyId))
Copy link
Collaborator

Choose a reason for hiding this comment

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

세터말고 emit한 이유가 뭔가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

update vs setter vs emit

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/72224263/mutablestateflow-value-vs-update-vs-emit

Copy link
Member Author

Choose a reason for hiding this comment

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

setter랑 emit 차이는 없는걸로 알고 있었는데 update는 첨보네요 굳

}
.onFailure {
_createStudyUiState.emit(CreateStudyUiState.Fail)
_createStudyEvent.emit(Event.CreateStudyFailure)
_createStudyState.emit(CreateStudyState.Failure)
}
}
}

private fun String.isNotBlankAndEmpty(): Boolean = isNotBlank().and(isNotEmpty())
private fun String.isNotBlankAndEmpty(): Boolean = isNotBlank() and isNotEmpty()

sealed interface Event {
object CreateStudySuccess : Event
object CreateStudyFailure : Event
data class NavigateToBefore(val fragmentState: FragmentState) : Event
data class NavigateToNext(val fragmentState: FragmentState) : Event
}
Comment on lines +130 to +135
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 참고했던 구글의 프로젝트에선 Event가 아닌 Action으로 정의하고
아래 처럼 Action(행동)에 관한 Navigate관련 클래스 두개로만 관리하긴 합니다.

sealed class OnboardingNavigationAction {
    object NavigateToMainScreen : OnboardingNavigationAction()
    object NavigateToSignInDialog : OnboardingNavigationAction()
}

현재 코드는 행동과 이벤트(상황)가 혼합된 느낌이긴 합니다.
분리하는 것에 대해 어떻게 생각하시나요?


sealed interface CreateStudyState {
class Success(val studyId: Long) : CreateStudyState

object Loading : CreateStudyState

object Failure : CreateStudyState

object Idle : CreateStudyState
}
Comment on lines +130 to +145
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래 state에서는 어느 state인지 CrateStudyState라고 해주셨는데,
위의 Event는 그냥 Event라고 명시해주셨네요.

서로 다른 이유가 혹시 있을까요?


companion object {
private const val DEFAULT_INT_VALUE = -1
Expand Down
Loading