[FEAT/#14] 7차 필수과제 #15
Conversation
Hyobeen-Park
requested review from
jihyunniiii,
MinseoSONG,
beom84 and
wjdrjs00
| class PreferenceImpl( | ||
| @ApplicationContext private val context: Context | ||
| ) { | ||
| private val preference = context.getSharedPreferences( | ||
| PREF_NAME, Context.MODE_PRIVATE | ||
| ) | ||
|
|
||
| var token: String | ||
| get() = preference.getString(TOKEN, "").toString() | ||
| set(value) = preference.edit().putString(TOKEN, value).apply() | ||
|
|
||
| companion object{ | ||
| private const val PREF_NAME = "wavve_prefs" | ||
| private const val TOKEN = "token" | ||
|
|
||
| val LocalPreference = staticCompositionLocalOf<PreferenceImpl> { | ||
| error("Preference Failed") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이 친구에 대해 자세히 설명해주실 수 있을까요 ?? sharedpreference를 관리하는 친구인거같은데 동작법이 궁금합니다 !
| CompositionLocalProvider( | ||
| PreferenceImpl.LocalPreference provides PreferenceImpl(this) |
| sealed interface MySideEffect : UiSideEffect { | ||
|
|
||
| } | ||
|
|
||
| sealed class MyEvent : UiEvent { | ||
|
|
||
| } |
There was a problem hiding this comment.
sideeffect와 event가 없어도 일단은 만들어두는게 좋은걸까요 ? 저는 안만들엇거등요
| data class OnUsernameChanged(val username: String) : SignInEvent() | ||
| data class OnPasswordChanged(val password: String) : SignInEvent() | ||
| data object OnSignInButtonClicked : SignInEvent() | ||
| data object OnSignUpButtonClicked : SignInEvent() |
There was a problem hiding this comment.
대현님께도 여쭤봣지만 네이밍에 On이 붙는게 일반적인걸까요 ??
|
|
||
| LaunchedEffect(true) { | ||
| viewModel.getHobby(sharedPreferences) | ||
| viewModel.getHobby(preference.token) |
There was a problem hiding this comment.
저도 이번에 mvi패턴을 처음 다뤄봐서 확신은 없지만,, 이부분을 sideEffect로 처리해야하지 않나? 하는 생각이 들었습니다!
|
|
||
| } | ||
|
|
||
| fun getHobby(token: String) { |
There was a problem hiding this comment.
요 부분도 위 리뷰랑 연결되어서 sideEffect로 처리하고 private가시성을 사용할 수 있지 않나? 하는 생각이 들긴 하는데,, 단순 생각입니다!! mvi 어려워서 아직 저도 확신은 안생깁니다 ㅎㅎ,,
| context.toast(sideEffect.message) | ||
| } | ||
|
|
||
| is SignInSideEffect.ShowSnackBar -> {} | ||
| is SignInSideEffect.NavigateToSignUp -> { | ||
| is SignInContract.SignInSideEffect.NavigateToSignUp -> { |
There was a problem hiding this comment.
mvi패턴에서는 추가 로직이 없는 단순 화면 이동같은 경우도 sideEffect로 처리해야하나요??
There was a problem hiding this comment.
core에 위치시키신 이유가 궁금합니다 !
| if (password.contains(NUMBER_REGEX.toRegex())) count++ | ||
| if (password.contains(SPECIAL_CHAR_REGEX.toRegex())) count++ | ||
| is SignUpContract.SignUpEvent.OnHobbyChanged -> { | ||
| val isSignUpEnabled = isSignUpAvailable() |
There was a problem hiding this comment.
이 값을 인자로 받아와서 handleEvent 함수에서는 단순히 state를 업데이트 시키는 로직만 담당할 수 있도록 만들어주어도 좋겠네요.
Related issue 🛠
Work Description ✏️
Screenshot 📸
ui 바뀐 것 없습니다!
Uncompleted Tasks 😅
이것도 없어용!!
To Reviewers 📢
preference를 급하게 만드느라 screen과 viewModel에 마구 섞여있는게 신경쓰였었는데 드디어 수정했습니다!! 이게 맞는 방법인지는 모르겠지만.. 확실히 전보다는 나은 것 같아요ㅎㅎ
MVI 저도 잘 못하니까 코리 마구마구 달아주세요!