Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/#916 refactor datalayer refactoring #917

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

chws0508
Copy link
Collaborator

@chws0508 chws0508 commented Mar 3, 2024

#️⃣ 연관된 이슈

close : #916

📝 작업 내용

Data Package Refactoring

  • Service Factory 삭제
  • Retorfit 과 OkHttp관련 코드를 DI로 이동
  • Network 패키지와 Persistence 패키지로 구분
  • DI패키지를 삭제하고 관심사와 맞는 패키지에 DI로직을 이동
  • Model을 공통 패키지로 이동

스크린샷 (선택)

예상 소요 시간 및 실제 소요 시간 (일 / 시간 / 분)

예상 소요 시간 : 2시간
실제 소요 시간 : 2시간

💬 리뷰어 요구사항 (선택)

  • 이상 있는지 다 테스트결과 앱 잘 돌아갑니다.
  • 이 브렌치 체크아웃 해보고 폴더 느낌 함 봐주세요. 훨씬 폴더구조가 깔끔하고 이제 요구사항이 생겼을 때, 어디를 수정해야 할지 감이 올 것입니다.
  • 욕심상 도메인 패키지를 만들고 클린아키텍쳐로 만들고 싶지만 일단은 이렇게 하겠습니다.

@chws0508 chws0508 added Android 안드로이드 관련 이슈 리팩터링 테스트 코드의 검증 값이 변환하지 않고 코드 변경 labels Mar 3, 2024
@chws0508 chws0508 requested a review from ki960213 March 3, 2024 19:08
@chws0508 chws0508 self-assigned this Mar 3, 2024
@tmdgh1592
Copy link
Member

tmdgh1592 commented Mar 3, 2024

고생하셨습니다!
전체적으로 패키지 구조가 명확해졌다는 느낌이 들어요.
이전에는 data 패키지 하위에 너무 많은 디렉토리가 있었는데, 지금은 경우에 따라 잘 묶이고 분리된 것 같아요!

+) 코멘트를 드리자면 프로젝트에 Module이 그렇게 많지 않은데 di 관련된 파일들이 너무 분산되어 있다고 느꼈어요.
특정 모듈을 수정하기 위해 여러번 클릭해서 들어가는게 번거롭다고 느껴졌습니다..!
di 패키지는 하나로 모아 놓는 것은 어떨까요??

Copy link
Collaborator

@ki960213 ki960213 left a comment

Choose a reason for hiding this comment

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

확실히 더 잘 나뉜 것 같아요. 👍
저는 지금처럼 Hilt의 모듈이 모듈에서 제공하는 것과 가까이 있는 게 더 나은 것 같아요. 예를 들어 Repository를 추가할 때 di 패키지로 이동하는 것보다 Repository 패키지 안에서 수정하는 게 더 편할 것 같아서 그렇습니다.

Comment on lines 185 to 186
// datastore
implementation("androidx.datastore:datastore-preferences:1.0.0")
Copy link
Collaborator

Choose a reason for hiding this comment

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

불필요한 의존성은 지워도 될 것 같아요!

@chws0508
Copy link
Collaborator Author

chws0508 commented Mar 4, 2024

고생하셨습니다! 전체적으로 패키지 구조가 명확해졌다는 느낌이 들어요. 이전에는 data 패키지 하위에 너무 많은 디렉토리가 있었는데, 지금은 경우에 따라 잘 묶이고 분리된 것 같아요!

+) 코멘트를 드리자면 프로젝트에 Module이 그렇게 많지 않은데 di 관련된 파일들이 너무 분산되어 있다고 느꼈어요. 특정 모듈을 수정하기 위해 여러번 클릭해서 들어가는게 번거롭다고 느껴졌습니다..! di 패키지는 하나로 모아 놓는 것은 어떨까요??

저도 토마스 의견처럼 Hilt의 모듈이 모듈에서 제공하는 것과 가까이 있는 게 추후 유지보수와 관심사 분리 측면에서 좋아 보입니다

@chws0508 chws0508 merged commit 090f2fb into android-main Mar 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android 안드로이드 관련 이슈 리팩터링 테스트 코드의 검증 값이 변환하지 않고 코드 변경
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants