Conversation
hyeeum
requested review from
1971123-seongmin,
HAJIEUN02,
jihyunniiii and
kamja0510
and removed request for
1971123-seongmin and
kamja0510
|
시험 끝나고 코드 더 변경하시면 그때 한번에 코리할게요!_! |
1971123-seongmin
approved these changes
Dec 18, 2024
Member
1971123-seongmin
left a comment
1971123-seongmin
left a comment
There was a problem hiding this comment.
과제 하시느라 고생하셨어요. 시험 힘내세요 🥲🥲
greedy0110
reviewed
Dec 18, 2024
Comment on lines
+20
to
+22
| abstract fun bindsSignInRepository( | ||
| signInRepositoryImpl: SignInRepositoryImpl | ||
| ): SignInRepository |
There was a problem hiding this comment.
Repository랑 RepositoryImpl을 구분하는 이유는 무엇인가요? 어차피 구현체 하나라서 인터페이스 없이 Repository를 구체 클래스로 제공해도 무방할 것 같아서요
greedy0110
reviewed
Dec 18, 2024
| import org.sopt.and.domain.repository.MyRepository | ||
| import javax.inject.Inject | ||
|
|
||
| class MyUseCase @Inject constructor( |
There was a problem hiding this comment.
이 객체는 아래처럼 쓰일 텐데요.
val myUseCase = MyUseCase(...)
val ?? = myUseCase()무엇을 하는 UseCase인지 알기 어려워 보입니다.
GetHobbyUseCase라고 하심은 어떠신지요?
더 나아가서 이 UseCase는 단순히 하나의 Repository 함수 호출을 감싸는 수준에 불과합니다.
MyUseCase 객체를 관리해서 얻는 이점이 뭐가 있나요? 그냥 ViewModel에서 Repository 객체를 받아서 사용하면 되는 것 아닌가요?
There was a problem hiding this comment.
UseCase들의 이름과 필요성에 대해서 고민이 필요합니다. 다른 UseCase들도 다요.
jihyunniiii
reviewed
Dec 25, 2024
| fun getUserHobby() { | ||
| viewModelScope.launch { | ||
| wavveRepository.getHobby().onSuccess { hobbyEntity -> | ||
| getHobbyUseCase.invoke().onSuccess { hobbyEntity -> |
Contributor
There was a problem hiding this comment.
Suggested change
| getHobbyUseCase.invoke().onSuccess { hobbyEntity -> | |
| getHobbyUseCase().onSuccess { hobbyEntity -> |
이렇게도 사용할 수 있을 것 같아요.
invoke 함수가 무엇인지 공부해 봅시다.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
아직 6주차 PR을 안닫아서 끌어당겨 와서 PR을 올렸습니다 :)
생각해보니...그냥 병합했으면 되는 PR이네요.. 15번 이슈번호가 있는 PR부터 봐주시면 될 것 같습니다
급하게 하느라 아직 제대로 MVI를 반영하진 않은 것 같습니다.. 시험 목요일에 끝나서 얼른얼른.. 코드리뷰 마감전까지는 수정할 수 있도록 하겠습니다..!! 죄송합니다 ㅠㅠ