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

[FEAT/#11] 7주차 과제 #12

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

[FEAT/#11] 7주차 과제 #12

wants to merge 10 commits into from

Conversation

yskim6772
Copy link
Contributor

Related issue 🛠

Work Description ✏️

  • MVI 패턴 적용

Screenshot 📸

Uncompleted Tasks 😅

  • [ ]

To Reviewers 📢

7주차 과제 조금 늦었습니다 .. 죄송합니다 ..
리뷰 달아주시면 적극 반영하겠습니다 ! 아직 MVI를 제대로 적용한건지 모르겠네요 ..

@yskim6772 yskim6772 added the enhancement New feature or request label Dec 19, 2024
@yskim6772 yskim6772 self-assigned this Dec 19, 2024
Copy link

@gitsuhyun gitsuhyun left a comment

Choose a reason for hiding this comment

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

sideEffect, event 적용을 잘 해주신 것 같아요!! 요래저래 많이 보고 mvi 이해해봐야겠어용ㅋㅅㅋ

import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch

abstract class BaseViewModel<State : UiState, SideEffect : UiSideEffect, Event : UiEvent> : ViewModel() {

Choose a reason for hiding this comment

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

baseViewModel을 추가해서 sideEffect와 event를 쉽게 관리해줄 수 있군용

Comment on lines +26 to +31
override suspend fun handleEvent(event: MyPageEvent) {
when (event) {
is MyPageEvent.LoadUserHobby -> {
val token = sharedPreferences.getString(TOKEN, "").orEmpty()
if (token.isNotEmpty()) {
getHobby(token)

Choose a reason for hiding this comment

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

이런식으로 활용할 수 있구낭..!!

import org.sopt.and.core.component.UiSideEffect
import org.sopt.and.core.component.UiState

class MyPageContract {

Choose a reason for hiding this comment

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

contract 깔끔하네용

Copy link

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.

Comment on lines +51 to +53
hilt {
enableAggregatingTask = false
}

Choose a reason for hiding this comment

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

요 코드가 왜 작성되어야 하는지 어떻게 하면 작성하지 않아도 되는지 공부해보시면 좋을 것 같아요

Choose a reason for hiding this comment

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

core에 위치시키신 이유가 무엇인지 궁금해요 !

val hobby = response.result.hobby
if (hobby.isNotEmpty()) {
_hobby.value = hobby
override suspend fun handleEvent(event: MyPageEvent) {

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
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 7주차 과제
3 participants