Feat/local-db #9
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
jinsu4755
approved these changes
Jan 11, 2024
| import org.care.packie.domain.CategoryRepository | ||
| import javax.inject.Inject | ||
|
|
||
| class CategoryRepositoryImpl @Inject constructor( |
There was a problem hiding this comment.
개인적으로 Impl 자체의 네이밍은 확장성에 좋지 않다고 생각해!
예를 들어서 현재 컴포즈에서 Preview 를 위한 fake 데이터를 불러오기 위해서 FakeCategoryRepositoryImpl 을 가지는 것보다
각각
LocalCategoryRepository : CategoryRepository
FakeCategoryRepository: CategoryRepository
각 구현체가 정확하게 어떤 역할을 구현중인지 내포하면 좋을 것 같아!
There was a problem hiding this comment.
약간 예를 들자면 저장소로 소통할 수 있는 명세(인터페이스)를 따르는 저장소를 각각
저장소 구현체, 저장소 구현체라고 하는 것보다
로컬데이터로 저장소로 소통할 수 있는 객체.
가짜데이터로 저장소로 소통할 수 있는 객체
정도로 보다 명확한 뜻을 가지는게 좋을 것 같아!
app/src/main/java/org/care/packie/feature/category/CategoryScreen.kt
Outdated
Show resolved
Hide resolved
| @@ -42,30 +46,42 @@ private const val MAX_SPACER_SIZE = 80 | |||
|
|
|||
| @Composable | |||
| fun CategoryScreen( | |||
There was a problem hiding this comment.
오 지금 보니까 이거 인자가 달라서 같은 이름으로 쓸 수 있구나
실제 Screen 이랑 뷰모델을 사용하는 Screen 이 같은 이름을 가지는건 네이밍 관련이라 큰 문제는 없지만
뷰모델을 사용하는 스크린을
XXScreenRoot 와 같은 이름을 사용하거나 해서 "순수 UI" 와 "로직을 감싸는" 각각의 역할을 내포하는이름이 있다면 더 좋을 것 같기도 하넹
나도 이건 고민해봐야긋다
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.
2024-01-11.10.25.48.mov