Skip to content

Conversation

Sangjin0702
Copy link

클린 아키텍쳐 어려워요..ㅜㅜ

Copy link
Contributor

@kongwoojin kongwoojin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.
UseCase도 잘 구현하셨고, hilt도 사용하려고 하신게 보이네요
다만, 아직 몇가지 문제점이 보입니다.
먼저, repositoryImpl이 domain에 있는데, 이러면 인터페이스 패턴을 사용한 이유가 없어집니다. impl은 data layer에 들어가야 합니다.
또한, repositoryImpl에 Pager를 사용하셨는데, Pager 또한 안드로이드 의존성을 가지는 라이브러리입니다. 하지만 워낙 말이 많은 애라서 이건 알고만 계시면 될 것 같고요
Repository와 RepositoryDto 두개 모두가 지금 data에 들어가 있는데, 전자는 domain layer에 가는게 맞아 보이네요
그리고 보통 서버에서 값을 받아오는 dto는 이름 뒤에 Response를 붙여줍니다. 이 경우에는 RepositoryResponse가 되겠네요

다음번에는 모듈 분리를 해보시면 좋을 것 같습니다.

Copy link

@KYM-P KYM-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

너무 늦게 리뷰를 달아드려서 죄송합니다.
다만 더 똑똑한 킹 갓 우진님의 리뷰를 대신 받았으니 더 좋으실 겁니다.
클린 아키텍쳐는 어렵습니다. 저도 잘 모르는거 같아요.

lifecycleScope.launch {
viewModel.repositories.collectLatest { pagingData ->
binding.progressBar.isVisible = true
adapter.submitData(pagingData = pagingData)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이부분은 비동기 실행으로 알고있습니다 즉 결과값이 다 반환되기 전에 progressbar.visible 이 false 가 되어버립니다.
근데 앞서 adapter loading flow에 따라 progressbar를 보여주고 있어서 progressbar.visible을 쓸 필요가 없는게 아닌지 생각이 드는군요

@@ -0,0 +1,7 @@
package com.example.bcsd_android_2025_1.data

data class Repository(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우진님과 중복이지만 이런 데이터 객체의 경우
data 뿐만이 아닌 presentation 에서도 쓰기에 domain 에 있어야할 거 같습니다. (그리고 보통 domain 에 만듭니다.)


data class RepositoryDto (
val id: Long,
val name: String,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

명칭이 같으면 어노테이션을 안넣어도 되긴 합니다.
다만 Koin 에서는 확인차 넣고있기에 넣어보시는걸 추천합니다.

import com.example.bcsd_android_2025_1.data.toDomain


class GithubRepositoryImpl@Inject constructor(private val api: GithubApi) : GithubRepository{
Copy link

@KYM-P KYM-P Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 중복이겠지만 impl는 data에 있어야합니다.
domain 은 실질적으로 api 호출방법등과 같은 정보를 몰라야 하기 때문이죠.
domain은 단지 버튼을 누르면 함수 A를 실행한다 정도의 정보만 가지고 실제로 A의 기능 동작부분은 data에 구현합니다. (생산직과 관리직 구분..? 아니면 중대장과 병사의 관계?)
우선 저는 이렇게 기억하는 중입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants