최근 추가된 노래 클릭시 플레이어 화면으로 이동 #272
Conversation
| ) | ||
| } | ||
| } | ||
|
|
||
| fun interface Listener { |
There was a problem hiding this comment.
Listener 네이밍, 더 명확히 작성하시면 어떨까요?
아니면 MusicAdapter 생성자 변수 'listener'를 더 명확히 해도 좋습니다. 둘 중 하나는 하면 좋을 거 같아요
class MusicAdapter(
private val musicItemOrientation: Orientation,
private val listener: Listener = Listener { }, // todo : 클릭 이벤트 구현이 완료되면 디폴트 값을 지워주세요.
)There was a problem hiding this comment.
Listener는 중첩 인터페이스라 Adapter 내부에서는 Listener로 접근이 가능하지만, 외부에서는 MusicAdapter.Listener처럼 접근해야 돼. media3 라이브러리의 리스너도 Player.Listener처럼 작성되어서 리스너라는 이름만 사용했어. 혹시 피드백이 이 내용이 맞을까?
There was a problem hiding this comment.
그런데 클릭 이벤트 처리는 data class의 람다로 처리한다고 해서, 이 코드는 아마 바뀔 것 같아
There was a problem hiding this comment.
binding.rvRecentlyAddedSong.adapter = MusicAdapter(
musicItemOrientation = Orientation.HORIZONTAL,
listener = viewModel,
)코멘트 남긴 이유는 listener = viewModel, 경우 처럼, 읽는 사람 입장에서 너무 추상적이여서 입니다~
바뀌면 상관 없습니다~
There was a problem hiding this comment.
업캐스팅하거나, 자동 타입 캐스팅을 이용하는 경우가 흔한 것 같아서 저는 괜찮다고 생각해요.
this는 activity이지만 실제 파라미터는 context인 것처럼요.
There was a problem hiding this comment.
흐음... 오늘은 좀 지쳐서 이슈 만들어 놓고 다음에 적용할게요 ㅠㅠ
| player.clearMediaItems() | ||
| player.setMediaItems(newItems) | ||
|
|
||
| val index = newPlaylist.musics.indexOf(newPlaylist.startMusic) |
There was a problem hiding this comment.
usecase 에서 index 를 구하고, playlist 랑 start index 를 넘겨주면 이 코드는 presentation 에 없어도 될 것 같네요!
There was a problem hiding this comment.
UseCase에서는 "일단 많은 정보를 포함하고, 어떻게 사용하는지는 신경쓰지 않는다"라는 의도로 설계했어요.
7~8주차 페어프로그래밍 때 처음엔 id만 전달했다가 나중에 객체 전체를 전달하는 방식으로 변경되었는데요. 객체를 전달하는 편이 나중에 변경 사항이 생겼을 때 대응하기 더 쉬울 것 같은데, 어떻게 생각하시나요?
There was a problem hiding this comment.
아 객체를 전달하는 것 좋아요 ! 객체는 그대로 두고 index는 멤버변수를 생각했습니당
Issue
Overview
To Reviewers