Skip to content

[CHORE/#177] 홈 뷰 패키지 구조 수정#192

Merged
Hyobeen-Park merged 9 commits intodevelopfrom
chore/#177-home-packaging
Aug 26, 2024
Merged

[CHORE/#177] 홈 뷰 패키지 구조 수정#192
Hyobeen-Park merged 9 commits intodevelopfrom
chore/#177-home-packaging

Conversation

@Hyobeen-Park
Copy link
Copy Markdown
Member

⛳️ Work Description

  • domain entity 이름 수정 및 feature별로 패키지 분리
  • mapper 함수 분리

📸 Screenshot

뷰 수정 없습니다!

📢 To Reviewers

  • 파일 이동만 및 이름 수정만 진행했습니다
  • 리팩토링 한거는 따로 이슈 파서 pr 날릴게요😂

@Hyobeen-Park Hyobeen-Park added CHORE 🎀 버전 코드 수정, 패키지 구조 변경, 타입 및 변수명 변경 등의 작은 변경 효빈💚 효빈 labels Aug 21, 2024
@Hyobeen-Park Hyobeen-Park added this to the 2차 스프린트 작업 milestone Aug 21, 2024
@Hyobeen-Park Hyobeen-Park self-assigned this Aug 21, 2024
Copy link
Copy Markdown
Member

@leeeyubin leeeyubin 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 +35 to 37
).result.map { homeRecommendInternResponseDto ->
homeRecommendInternResponseDto.toHomeRecommendIntern()
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

오홍,, 저도 몰라서 찾아봤는데 map함수를 써준 이유는 리스트로 반환하기 위해서군요..?
그럼 위의 toHomeTodayInternList()함수명이랑 통일해주는 건 어떤가용,,?
예를들어 toHomeRecommendInternList() 라던지,,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

섬세하닷.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

헉!! 둘이 통일이 안돼있었네요..!! 섬세하닷. 감사합니다~~!!

Copy link
Copy Markdown
Contributor

@arinming arinming 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 +35 to 37
).result.map { homeRecommendInternResponseDto ->
homeRecommendInternResponseDto.toHomeRecommendIntern()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

섬세하닷.

viewModel.updateSelectColor(newColor)
},
homeRecommendInternModel = HomeRecommendInternModel(
homeRecommendIntern = HomeRecommendIntern(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

요 모델이,, 검색 뷰에서도 쓰이는데 네이밍에 Home이 들어가도 될까용..?! 흠

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

헉스 검색 뷰에도 사용되는걸 놓쳤네요...!! 확인해보니 검색뷰 모델은 따로 있고, HomeRecommendInternDialog를 사용하면서 홈쪽 모델도 같이 사용하게 된 것 같은데 맞을까요...?? 이 부분은 데이터 형식 때문에 dialog까지 따로 만들게 된거라 데이터 형식 통일되고 dialog 분리할 때 같이 분리하면 좋을 것 같아요!!

Copy link
Copy Markdown
Member

@boiledeggg boiledeggg 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 56 to 57
navController,
viewModel,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

뷰모델이나 네비게이션 객체를 인자로 넘겨주는 것보단 고차함수로 넘기는 게 좋을 것 같아요!!
유빈이가 깔끔하게 해놨던데 참고해봐요~~!

Comment on lines 78 to 80
navController: NavController,
viewModel: HomeViewModel,
) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

바로 밑에 코드에 코멘트가 달리지 않아 여기에 답니다ㅎㅎ

지금 보니까 뷰모델의 상태값을 인자로 받아온 상태에서 밑에 remember를 사용해서 변수를 선언했는데 왜 이렇게 구현한건가요??
처음 봤을 땐 뭔가 의아했는데 보면 볼수록 의미있는거 같고 그렇네요ㅋㅋ

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

일단 navController는 함수로 넘기는 방식으로 변경했고, viewModel같은 경우는 현재 년/월과 필터링 재설정 API 함수때문에 viewModel을 그대로 같이 넘겼던 것으로 기억하는데 다른 더 좋은 방법이 있을 것 같아서 계속 고민중입니다!!

val homeRecommendInternList = when (homeRecommendInternState) {
is UiState.Success -> {
(homeRecommendInternState as UiState.Success<List<HomeRecommendInternModel>>).data
(homeRecommendInternState as UiState.Success<List<HomeRecommendIntern>>).data
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

홈화면에서 목록을 밑으로 스크롤한 상태에서 스크랩을 하면 화면이 다시 로드되면서 위로 되돌아가는 문제가 고쳐졌는지 모르겠지만,
만약 아직도 문제가 된다면 제가 예상하는 원인은 다음과 같아요:

  • 뷰모델의 코드, LaunchedEffect 그리고 상태 확인 코드들간의 의존성이 엮여서 복잡하고 불필요한 호출이 많이 일어나는 것 같음
  • 각 부분의 데이터가 바뀌면 관련 컴포저블들만 리컴포지션되는 것이 아니라 HomeRoute 전체에 대한 리컴포지션되고 하위 데이터들이 바뀌는 것 같음

지금 이렇게 최상위 컴포저블에서 모든 상태를 관리하는 방법에서 상태값이 바뀌면 관련된 컴포저블을 업데이트하는 방식으로 바꿔보는 건 어떨까 싶어요!!

즉, When문을 사용해 homeRecommendInternList와 같은 변수를 선언/초기화하여 HomeScreen의 인자로 넘기는 것이 아니라, When문을 사용해 Success 상태일 때 HomeScreen/LazyColumn/items/RecommendInternItem 컴포저블을 보여주는 방향으로 바꿔 보세요!!

이게 해결방법이 아닐 수는 있지만, when문에서 뷰를 업데이트하는게 더 옳은 방법이라고 생각합니다!!
ChangeFilterRoute에서 하신 것처럼 해주시면 됩니다!!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

우왕 정말 감사합니다!!! 정말 확인해보니 HomeRoute 전체가 리컴포지션되더라구요😭 말씀해주신 방식으로 수정해보겠습니당 진짜 최고최고💚

@Hyobeen-Park Hyobeen-Park merged commit 6778e36 into develop Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CHORE 🎀 버전 코드 수정, 패키지 구조 변경, 타입 및 변수명 변경 등의 작은 변경 효빈💚 효빈

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CHORE] 홈 뷰 패키지 구조 수정

4 participants