Fix/코드리뷰#37
Hidden character warning
Conversation
- 클래스와 객체의 이름은 대문자 카멜 표기법을 사용
|
|
||
| // UI data class | ||
| // - API data 로 받아온 걸 UI data 형태로 데이터 변환(Mapping) 해서 표현되게 해줘야 함 | ||
| data class LetterUiModel( |
There was a problem hiding this comment.
data layer 의 폴더 안에 UI model이 있는 게 조금 이상한 것 같아요.
나중에는 멀티 모듈로 개선하면 좋을 것 같습니다.
| // Hilt에 결합 정보를 제공하는 한 가지 방법은 생성자 삽입입니다 | ||
| // 클래스의 생성자에서 @Inject 주석을 사용하여 클래스의 인스턴스를 제공하는 방법을 Hilt에 알려줍니다 | ||
| @Singleton | ||
| class ChattingRepository @Inject constructor( |
There was a problem hiding this comment.
Repository에 Singleton annotation을 붙여야 할까요?
| counselorId: String, | ||
| page: Int, | ||
| size: Int | ||
| ): Response<ChattingRoomInfo> { |
There was a problem hiding this comment.
Response 반환이 아닌 ChattingRoomInfo 로 반환 시켜보세요. Response로 반환하게 되면 해당 repository 에 대한 사용이 너무 어려울 것 같습니다.
| import com.simply407.patpat.data.model.CounselorMbtiInfo | ||
|
|
||
| // TODO 샘플코드이긴 하지만... 만약 String을 객체로 담고 싶다면?? | ||
| class MBTIRepository(context: Context) { |
There was a problem hiding this comment.
사실 이렇게 context를 넣어서 관리하는 것 보단 Resource만 보내고 이걸 사용하는 UI component쪽의 context를 사용해서 해당 Resource를 사용하는게 더 좋을 것 같습니다.
|
|
||
| override fun getItemViewType(position: Int): Int { | ||
| return when (getItem(position).role) { | ||
| "ASSISTANT" -> TYPE_COUNSELOR |
There was a problem hiding this comment.
API의 Response 의 role이라는 field로 String type이 나와서 이렇게 정의 하신 것 같은데, Enum으로 받을 수 있습니다. 어떻게 해야 Enum으로 관리할 수 있을지 고민해보세요.
| } | ||
|
|
||
| companion object { | ||
| const val TYPE_COUNSELOR = 1 |
There was a problem hiding this comment.
내부에서만 쓰이는건 private 을 붙여주세요. 개발하실때 모든 변수는 private을 붙이는 습관을 들여주세요.
| } | ||
|
|
||
| // viewModel 에서 가져와서 observe patten 으로 갱신(submitList) 하도록 하는게 더 좋음 | ||
| fun submitNewMessages(newMessages: List<MessageInfo>) { |
There was a problem hiding this comment.
굳이 이걸 만들 필요가 있을까요? submitList로 그냥 쓰는게 좋을 것 같습니다.
2 participants
No description provided.