[FEAT/#357] Profile / 프로필 수정 뷰 중고딩 부분 구현 #366
Merged
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
Marchbreeze
added
FEAT ✨
minju1459
approved these changes
Feb 2, 2024
| object Success : ImageChangeState() | ||
| object NotChanged : ImageChangeState() | ||
| object Error : ImageChangeState() | ||
| } |
b1urrrr
approved these changes
Feb 4, 2024
| object Success : ImageChangeState() | ||
| object NotChanged : ImageChangeState() | ||
| object Error : ImageChangeState() | ||
| } |
| if (viewModel.grade.value != grade.toString()) { | ||
| viewModel.grade.value = grade.toString() | ||
| viewModel.isChanged = true | ||
| (activity as SchoolProfileModActivity).checkIsGradeTextNone() |
There was a problem hiding this comment.
저도 이전 앱잼 멘토분께 받았던 피드백인데, as를 활용하여 강제 타입 변환을 시키는 것은 crash를 야기할 수 있어 위험할 수 있는 습관이라고 합니다! 대체할 수 있는 방법을 생각해보면 어떨까요?
EX) 고차함수, 공유 뷰모델
There was a problem hiding this comment.
이것도 리팩토링하면서 고쳐보겠습니다 ! 액티비티를 액티비티로 타임 지정을 해주는 방법이라 괜찮다고 생각했는데, 혹시 모를 오류가 발생할 수도 있겠군요..!
| _getSchoolListState.value = UiState.Empty | ||
| } | ||
|
|
||
| fun getUserDataFromServer() { |
There was a problem hiding this comment.
민주 PR에도 달았던 코리를 데려왔습니다!
뷰모델은 리포지토리의 함수가 서버 통신과 관련되었는지 로컬 디비와 관련되었는지 알 필요가 없기 때문에 네이밍에도 FromServer 같은 건 빼주는 게 추상화의 효과도 극대화되고 유지보수성이 향상되지 않을까요?
꼭 써주고 싶다면 차라리 리포지토리 함수에 명시하는 게 좋을 것 같다는 생각입니다...
There was a problem hiding this comment.
아핫...! 함수 만들때 우리 서버랑 카카오랑 구분하려고 사용하던 습관이었슴니닷
담에 한번에 쏵 리팩토링 해볼겡요 ㅎㅎ
…into feat/#357-profile-mod-school
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.
⛳️ Work Description
📸 Screenshot
KakaoTalk_Video_2024-02-02-18-28-09.mp4
📢 To Reviewers