부산대 Android_이지은 5주차 2단계(리팩토링 완료) #77
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…se를 사용하여 데이터를 관리하도록 변경 - SavedLocation에 id 속성을 추가하기 위해서 Location에도 id 속성 추가(Kakao api에서 id 값을 가져온다) - Location을 객체 직렬화 후 intent로 클래스를 전달하도록 수정
…ocationRepository로 이동
- 5주차 2단계 기능 목록 작성
bigstark
requested changes
Jul 27, 2024
There was a problem hiding this comment.
고생하셨습니다 지은님! 전반적으로 좋은 방향으로 코드가 좋아지고 있는 것 같아요! 안드로이드 아키텍쳐 과 coroutine best practices 등 조금 더 참고하여 조금씩 변경해주시면 좋을 것 같아요!!
data binding 사용을 전반적으로 변경해주셔서 좋은 것 같아요! ViewHolder bind 시에 setOnClickListener 를 하는 것만 변경해주시면 좋을 것 같습니다 :)
| inner class LocationHolder( | ||
| itemView: View, | ||
| inner class LocationViewHolder( // | ||
| private val itemLocationBinding: ItemLocationBinding, |
Comment on lines
47
to
48
| } // -> item_saved_location.xml에서 data binding을 통해 ImageView에 리스너를 달았는데 작동을 안하네요..! 여기서 리스너를 달아도 괜찮은 걸까요? | ||
| } |
There was a problem hiding this comment.
안됩니다. bind 시에 setOnClickListener 를 추가하는 것은 좋지 않아요!
bigstark
requested changes
Jul 30, 2024
|
|
||
| @Singleton | ||
| @Provides | ||
| fun provideDispatchers(): CoroutineDispatcher { |
There was a problem hiding this comment.
CoroutineDispatcher 와 같은 abstract class 나 interface 는 다형성을 띄기 때문에 문제가 발생할 수 있어요.
Qualifer annotation 같은 것으로 "어떤 객체인지" 명확하게 보여주는 것이 좋습니다!
| itemView.setOnClickListener { | ||
| itemSelectedListener.onLocationViewClicked(item) | ||
| } | ||
| itemLocationBinding.onItemSelectedListener = itemSelectedListener |
There was a problem hiding this comment.
으악 ㅋㅋ 이것도 안하시는 것이 좋아요! 필요하다면 init 시점에서 처리해주시는 것이 좋습니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
중점적으로 봐주셨으면 하는 부분