Fix/calendar error #136
Merged
Fix/calendar error #136
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
HI-JIN2
reviewed
Mar 1, 2024
Comment on lines
+44
to
+45
| private var monthYearText: TextView? = null | ||
| private var calendarRecyclerView: RecyclerView? = null |
There was a problem hiding this comment.
별거는 아닌데, 요즘 리팩하면서 드는 생각이 이런경우에 ? =null을 하는게 맞을지, 아니면 non-nullable에 디폴트값(0이나 "")을 넣는게 나을지 고민입니다. 어떠한 방식이 나을까요?
There was a problem hiding this comment.
음 null을 넣어서 따로 처리하는 것 보다는 디폴트 값을 넣는 것이 데이터를 처리하는데 더 편할 거 같다고 생각이 드네요..!
HI-JIN2
reviewed
Mar 1, 2024
Comment on lines
80
to
81
| val intent = Intent(this, MyPageActivity::class.java) // 인텐트를 생성해줌, | ||
| startActivity(intent) // 화면 전환을 시켜줌 |
There was a problem hiding this comment.
요기 화면 이동도 util 함수로 있습니다! startActivity였던 것 같네용
HI-JIN2
reviewed
Mar 1, 2024
Comment on lines
31
to
34
| val layoutParams = view.layoutParams | ||
| layoutParams.height = parent.height | ||
| layoutParams.width = Gravity.RIGHT | ||
| return CalendarViewHolder(binding, view, onItemListener, days) |
There was a problem hiding this comment.
여기 view.layoutParams가 반복되어서 이렇게 한거라면, 스코프 함수 (apply, let 등)을 이용해보는건 어떠할까요~!
There was a problem hiding this comment.
오 apply 함수로 변경해서 반영해보도록 하겠습니다!
HI-JIN2
reviewed
Mar 1, 2024
|
|
||
| class CalendarViewModel : ViewModel() { | ||
| private val data = MutableLiveData<String>() | ||
| private val data = MutableLiveData<LocalDate>() |
There was a problem hiding this comment.
MenuFragment에서도 LocalDate를 사용해야 되니까 굳이 String으로 보낼 필요가 없는거 같아서 수정했습니다!
HI-JIN2
approved these changes
Mar 3, 2024
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.
관련 이슈
개요
작업 사항
Check List
ctrl + alert + O⇒ 안쓰는 import 제거 /ctrl + alert + L⇒ 코드 정렬 등)main브랜치의 최신 상태를 반영하고 있는지 확인스크린샷 및 작동 영상