-
Notifications
You must be signed in to change notification settings - Fork 70
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
[Android] han, funny조 코지와의 마지막 PR...... #272
base: team-01
Are you sure you want to change the base?
[Android] han, funny조 코지와의 마지막 PR...... #272
Conversation
val setDate = { text: String -> | ||
_date.value = text | ||
|
||
|
||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보통 명시적임 람다의 경우 dateSetter, onDateChange... 등등 일반 함수와는 구분하는 이름도 많이 사용하는 편입니다 .참고만 해주세요~
it.title = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) | ||
Html.fromHtml( | ||
"<font color='#FFFFFF'>${count}</font>", | ||
Html.FROM_HTML_MODE_LEGACY | ||
) | ||
else | ||
Html.fromHtml("<font color='#FFFFFF'>${count}</font>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 기능은 extension 으로 만들면 활용하기 좋은 편입니다!
_milestoneList.value[id].state = false | ||
val tempList = ArrayList<Milestone>() | ||
_milestoneList.value.forEach { | ||
if(it.state) tempList.add(Milestone(it.id, it.title, it.progress, it.content, it.date, it.open, it.closed, false,false, false, true)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
목록을 변환하는 경우 map 이 코드를 잘 정리해주는 편입니다!
binding.tvOpenIssueCount.text = binding.root.context.getString(R.string.open_issue_count_label, milestone.open) | ||
binding.tvClosedIssueCount.text = binding.root.context.getString(R.string.closed_issue_count_label, milestone.closed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viewHolder 클래스에선 itemView 가 접근 가능합니다.
itemView.context.getString() 등으로 줄일수 있으니 고려해주세요!
그리고 드린 리뷰중 stringResource 사용을 추천드렸던 사항은 xml 에서 바로 @string(resource, argument) 이렇게 사용하는 형태를 말씀드린거였습니다!
교육기간 동안 고생하셨습니다! 저도 즐거운 경험이었습니다:) 앞으로 열심히 하셔서 업계 동료로 뵈어요!
네 저희도 비슷한 방식을 사용합니다! 다른 방식으로는 Sdk 를 제공하는것도 있습니다. (전 sdk 를 선호하긴 합니다 ㅋㅋ)
음 큰 단점이 보이지는 않으나, realtimeData base 를 접근하는 방식이 뷰모델에서 같이 어우러지면 유지보수에는 더 좋겠네요!
바꾸는 것은 괜찮으나, Replace 를 사용하더라도 상태 복구는 가능합니다. 실제 navigation 라이브러리도 같은 방식으로 되어있고, fragmentManager 로도 구현이 가능하니 직접 학습해보시면 좋겠습니다! |
han & funny 조 마지막 PR입니다.
cozy 마지막 리뷰어로 좋은 조언 많이 해주셔서 감사했습니다 ㅠ
핑계지만 마지막 프로젝트에서 채용설명회와 이력서 작성 등 여러 프로그램들이 많아서 코드를 많이 못 보여드려서 너무 아쉽고, 죄송합니다.
그래도 질문에 좋은 답변과 조언해주셔서 저희 모두 코드에 대해서 더 고민하는 시간을 갖을 수 있었습니다.
다음에는 수강생이 아닌 개발자로 cozy의 리뷰를 받을 수 있도록 노력해보겠습니다.
바쁘실텐데 고생많으셨습니다! 감사합니다. 🙇🏻♂️
이번 PR은 저번에 viewModel 의존성과 bindingAdapter 말고 string resource로 변경해 보라고 하신 부분을 수정했습니다.
제가 이해를 잘 못해서 viewModel에 있는 부분을 view로 통째로 다시 옮기다가 아니란걸 깨닿고 다시 돌아 오느라 시간이 조금 걸렸습니다.
질문
현재 github 로그인 같은 경우 deepLink를 이용해서 jwt 발급에 필요힌 정보 받아와서 넘겨주는 형태인데 실제로 딥링크를 사용해서 redirect로 온 URI의 정보를 넘겨주는 방식을 많이 사용하나요? 아니면 다른 방식이 존재할까요?
firebase realtime database를 사용하려하는데 issue화면의 스피너에 필요한 데이터(milestone과 label, 작성자 등)가 MilestoneViewModel, LabelViewModel에 존재한다면 issueFragment에서 MilestoneViewModel, LabelViewModel을 참조해서 데이터를 받아오는 것은 별로 좋은 않은 행위일까요?
기존에 제가 구현 한 숫자 방식이 아닌 fragment의 이름으로 수정을 하니 가독성이 조금 더 좋아진 것 같습니다
다음으로 프래그먼트 별 백스택을 두는 방식으로도 구현해보려 합니다.
프래그먼트 학습 중 조금 다른 측면에서 질문이 생겨 남겨드립니다.
프래그먼트를 replace를 통해 교체하다 보니 새로운 프래그먼트가 생성되는 것으로 이해했습니다.
화면 상태, 데이터 등을 유지하기 위해 필요한 부분에는 show, hide 를 사용하는 것도 괜찮을까요???