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

[FIX] 코스 발견 / 스크랩 함수의 로직 수정 #325

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

leeeha
Copy link
Member

@leeeha leeeha commented Feb 14, 2024

📌 개요

✨ 작업 내용

저번 회의 때 외부 리사이클러뷰 어댑터에서는 마라톤, 추천코스 데이터 목록을 쥐고 있지 말자는 얘기가 나왔습니다. 이 데이터 목록들은 내부에 있는 각각의 리사이클러뷰 어댑터에서 처리를 해주는 것이 맞기 때문입니다. 그래서 외부 리사이클러뷰를 처음 초기화 할 때만 사용하는 방식으로 변경했습니다.

그런데 이때 외부 리사이클러뷰 어댑터의 스크랩 함수에서도 이 데이터 목록들을 사용하고 있다는 것을 깜박하여,,, 해당 부분까지 구현하여 다시 pr 올립니다. 저번에 더 꼼꼼히 확인했어야 했는데 이제야 발견해서 죄송합니다 🥹

✨ PR 포인트

이쪽 어댑터에서도 for문으로 아이템 탐색하는 것보다, 클릭한 아이템의 position 값을 어댑터에 저장해두는 게 나을까요?? 그러면 로딩 상태일 때는 이때 논의한 것과 마찬가지로 로딩 프로그레스바를 띄우는 등의 처리가 필요할 거 같습니다.

@leeeha leeeha added 하은 🐰 하은 담당 FIX 💥 버그 및 오류 해결 labels Feb 14, 2024
@leeeha leeeha self-assigned this Feb 14, 2024
Copy link
Member

@dongx0915 dongx0915 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 +116 to +118
multiViewHolderFactory.apply {
marathonCourseAdapter.updateMarathonCourseScrap(publicCourseId, scrap)
recommendCourseAdapter.updateRecommendCourseScrap(publicCourseId, scrap)
Copy link
Member

Choose a reason for hiding this comment

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

스크랩 할 때 왜 2개의 어댑터 모두 update 로직을 수행하는지 궁금합니다~!

MultiViewAdapter를 통해서 update 로직을 호출하기 때문일까요?
해당 이유라면 update 로직은 리스너 등으로 각 Adapter에 넘겨주는게 좋을 것 같기도 합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

원래는 아이템이 마라톤 코스인지, 추천 코스인지 is 연산자로 타입을 구분했었는데, 외부 리사이클러뷰 어댑터가 추천 코스의 전체 페이지 데이터를 갖지 않게 됨에 따라 이 방식이 불가능해졌습니다.

그래서 마라톤 코스와 추천 코스 어댑터에서 모두 update 함수를 실행시키고, publicCourseId와 일치하면 아이템을 갱신시키는 방식으로 구현해봤어요.

음 지금 생각나는 또 다른 방법은, 아이템을 클릭한 순간에 해당 아이템 타입이 무엇인지 (마라톤 or 추천코스) 람다식으로 넘기고 그 타입에 맞게 어댑터 함수를 실행시키는 방법이 있을 거 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

굳이 타입을 판별할 필요 없이 Adapter에서 클릭 리스너를 달아주면 되지 않을까요??

각 Adapter 내부에 update 로직을 넣어두고 클릭 이벤트 발생 했을때 호출해도 되지 않을까 싶은데 어떤가요??

마라톤 코스는 마라톤 코스 Adapter에서 할당한 클릭 리스너, 추천 코스는 추천 코스 Adapter에서 할당한 클릭 리스너로 각각 처리될테니 타입을 판별할 필요가 없다고 생각했습니다~!

Copy link
Member Author

Choose a reason for hiding this comment

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

어랏 지금 현재 그 방식으로 구현되어 있는 거 같은데요..?! 각 어댑터에서 update 함수 실행시키고 있습니다!

Copy link
Member

Choose a reason for hiding this comment

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

MultiViewAdapter에서 update를 호출하는 부분이 필요 없을 것 같다는 뜻이었으나 서버 응답 후에 처리 되어야하니 타입 판별이 필요하겠네요

좀 더 좋은 방법이 있는지 같이 논의 해보면 좋을 것 같습니다~!

Copy link
Contributor

@unam98 unam98 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 유사한 상황에 대해서는 방법을 통일하는 것이 좋을 것 같습니다!
나중에 더 좋은 방법을 찾았을 때 같은 logic으로 갈아끼우면 되니 더 편할 것 같아요!

@leeeha
Copy link
Member Author

leeeha commented Feb 14, 2024

@unam98 멤버변수를 만드는 방식으로 통일하려고 했는데, 문제 되는 부분이 있습니다. 왜냐하면 현재는 publicCourseId를 이용하여 이 아이템이 마라톤 코스인지, 추천 코스인지 구분하고 있습니다.

그런데 클릭된 item position을 저장해두고 서버 통신에 성공했을 때 해당 위치의 아이템을 갱신시키는 방식으로는,, 마라톤 코스, 추천 코스 이 두 가지 타입을 구분할 수 없습니다. 결국 다른 부분은 안 고치고 item position 변수를 만드는 방식만 적용하면, 마라톤과 추천 코스 리스트에서 동일 위치(인덱스)에 해당하는 아이템이 동시에 갱신될 것입니다.

따라서... for문으로 아이템 탐색 후 갱신시키는 현재 방법, 또는 어댑터의 update 함수 인자에 클릭된 아이템 타입이 무엇인지 같이 넘겨주는 방법이 있을 거 같습니다.

@leeeha leeeha merged commit 2dbb194 into develop Feb 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIX 💥 버그 및 오류 해결 하은 🐰 하은 담당
Projects
Development

Successfully merging this pull request may close these issues.

[FIX] 코스 발견 / 코스 스크랩 로직 수정
3 participants