[Android] 마지막 PR #283
[Android] 마지막 PR #283
Conversation
- Markwon 라이브러리 이용 마크원 라이브러리를 통해 텍스트 뷰 내용을 마크다운 형식으로 변환
[�Android] 이슈 작성 미리보기 기능
서버에서 구현되지 않아 메모리로 관리
[Android] 이슈 추가 화면 구현
- 이슈 상세 댓글 추가 - 키보드 컨트롤(숨김, 보여주기) - 댓글 커서로 움직이기(현재 위치) - 서버 통신 상세 DTO 변경 이슈 닫기 -> query를 status로 보내주기 때문에 불필요한 함수 제거
[Android] 이슈 상세 댓글 추가 및 네트워크 통신(상세조회, 이슈 닫기)
- data loading 속성을 추가해 데이터를 받아올 수 있도록 수정
| milestoneDto.description ?: "", | ||
| milestoneDto.deadline ?: "" | ||
| ) | ||
| issueTrackerApi.editMilestoneInfo(milestoneDto.id!!, editMilestoneDto) |
There was a problem hiding this comment.
id 가 비어있어서 예외가 발생하더라도, editMilestone 을 예외처리하고 있을테니 큰 문제는 없는데요,
실무에서는 !! 가 사용될때는 npe 로 묶어서 떨어지기 때문에 디버깅이 어려울 수 있습니다. (같은 라인에 !! 가 여러개 일 수 있음)
requireNotNull 등을 사용하면 예외에 대한 메시지 추가가 가능하니 알아봐주세요!
| fun setCurrentPosition(position: Int) { | ||
| val previousPosition = currentPosition | ||
| currentPosition = position | ||
| notifyItemChanged(previousPosition) | ||
| notifyItemChanged(currentPosition) | ||
| } |
There was a problem hiding this comment.
ListAdapter 는 DiffUtil 이 사용되므로 요런 position 에 대한 동작도 submit list 를 통해 구현해보시면 좋습니다!
| fun setOnRefreshListener(listener: () -> Unit) { | ||
| onRefresh = listener | ||
| } |
There was a problem hiding this comment.
혹시 fragment 바깥쪽에서 호출하는 함수라면, 프래그먼트 시나리오에 맞는 구현으로 변경해보시면 좋겠습니다.
Fragment 객체를 처음 만들때는 직접하지만, 재생성 시나리오에서는 fragmentManager 가 당담하기 때문에, refreshListener 도 초기화 될 수도 있습니다.
뷰모델을 이용하시거나, FragmentResult 등을 사용해보시면 좋겠습니다!
| _uiState.update { | ||
| val selectedFilters = it.selectedFilters.copy(assigneeIndex = index) | ||
| it.copy(selectedFilters = selectedFilters) | ||
| } |
duration api 는 하나의 예시였는데요, 숫자를 그대로 써놓는것은 시간이 지날수록 코드를 읽을때 탁탁 걸리는 부분이거든요. 그래서 최대한 이름이 추가되는것이 좋겠다고 생각한것입니다! 24, 30, 등이 MAX_DAY_HOURS 등으로 갈 수도 있겠죠?
기본 컴포넌트라 변경이 많지는 않겠지만, 리플렉션은 컴파일때 해당 함수가 없어진다고 해도 검사가 안되기 때문에 약간 잠재적 버그 느낌입니다. 저라면 팝업 메뉴 전체를 새로 구현했을거 같아요!
앱의 사용 시나리오마다 다르다고 생각합니다. 제가 일했던 서비스들에서는 상태라고 표현할 때 너무 다양해서.. (error,loading 등으로 전부 표현이 불가능) data class 하나로 놓고 업데이트하는것을 선호했습니다.
그 레이어가 어떤 것을 처리할지도 중요하지만, 프로젝트 전체에서 일관성을 유지하는게 조금 더 중요합니다. 정의하신 바가 그렇고 프로젝트 전체에서 잘 지키고 있다면 그게 좋은 상태입니다! 클린 아키텍쳐도 조금 공부하셔서 각 레이어들간에 지켜야 하는 규칙이 있을지도 고민해서 보시면 좋습니다. 해당 화면의 기능이 너무 적은 경우, viewModel을 생략하는것도 하나의 방법입니다. 특히나 요즘은 compose 가 잘 돌아가서요. |
안녕하세요. 코드스쿼드 마지막 코드리뷰를 받게된 @bn-tw2020 @banjjak2 입니다.
마지막 까지 최선을 다했습니다. 따끔한 피드백 부탁드리겠습니다.
💡 작업목록
✅ 이슈 상세 화면
이슈 댓글 추가
키보드 컨트롤(숨김, 보여주기)
댓글 커서로 움직이기
네트워크 서버 통신
이슈 닫기 -> query를 status로 보내주기 때문에 불필요한 함수 제거
✅ 이슈 등록 / 수정 화면
✅ 레이블 / 마일스톤 화면
✅ 피드백 반영
💡 질문
Q1. 코드 중복 줄이는 방법 여기서 예시로 들어주신거에서 실제로 적용하려니 어떻게 적용해봐야할지 엄두가 안나는데 도움을 얻어볼 수 있을까요?
Q2. duration api 여기에서
Duration.ofHours(1).seconds처러 상수말고 처리하라는 의도인지 여쭤봐도될까요?Q3. 리플렉션 이 코드에서는 팝업메뉴에서 아이콘을 넣고 싶었는데 적용이 안되서 검색을 통해 해결했습니다. 리플렉션을 학습하고 있으며, 일반적인 방법에는 어떤 처리가 있을까요?
Q4. 찾아보니 UI State를 저장하는 방법이 여러가지가 있더라구요. Label, Milestone에서 사용했던 data class로 상태를 저장하는 것과 Issue에서처럼 sealed class로 사용하는 것으로 각각 해보았는데 구현 및 적용해보면서 각각의 장단점을 알 수 있었던 것 같습니다. 리뷰어님께서는 어떤 방식으로 UI 상태를 저장하시는지 궁금합니다.
Q5. 네트워크 에러를 처리할 때 datasource 에서는 API 요청만 하고 repository에서는 결괏값에 대해 어떻게 처리할 것이며 viewModel에서는 결괏값을 사용자에게 어떻게 보여줄지 처리할 수 있도록 구현했습니다. 이 방법말고 더 좋은 방법이 있을지 궁금합니다.