Step3 - GitHub(UI) #106

Gyuil-Hwnag · 2023-08-31T16:53:56Z

안녕하세요~ 리뷰어님!
step3 적용 & step2 피드백 반영한 내용입니다!!
벌써 마지막 미션이네요. 잘부탁드립니다!!

작업 내용

앱을 실행하면 리스트 형태의 GitHub Repository UI가 보여야 한다. [8eb2b11] [82d250e] [71c80b0]
MVVM 패턴으로 구현한다.
Dagger2, Hilt를 이용하여 의존성을 주입한다. [e2c698f]
Domain 테스트 코드 추가 [768ac94] [e9b29b7]
ViewModel 테스트 코드 추가 [898559b]

// Response 가 nullable한 이유는 gitApi에서 description = null 인 경우가 있어서 리스트를 못받아오는 경우가 있어서 추가한 내용입니다!
internal data class RepositoryResponse(
    @SerializedName("id")
    val id: Int,

    @SerializedName("full_name")
    val fullName: String?,

    @SerializedName("description")
    val description: String?
)

step2 1차 피드백

Mapper 함수 최상단으로 끌어올리기 [3587ff6]
githubRepository.json 필요없는 부분 제거 [1dc3392]
테스트 코드 함수명 수정 [f9b1364]
GithubService 주석 수정 [e8caaf1]

wisemuji

안녕하세요 규일님!

Step3 구현을 잘 해주셨습니다 💯 테스트를 꼼꼼히 작성하려고 노력하신 부분이 인상적이네요.

전체적으로 군더더기 없이 구조를 설계해주셨습니다 👍
고민해보시면 좋을만한 사소한 코멘트를 달았으니 확인해주시면 감사하겠습니다.
다음 리뷰 요청에서는 머지하도록 할게요.

궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요! 🙏 💪

wisemuji · 2023-09-01T14:56:11Z

app/src/main/java/camp/nextstep/edu/github/ui/GithubActivity.kt

+        binding = ActivityGithubBinding.inflate(layoutInflater).apply {
+            viewmodel = viewModel
+            lifecycleOwner = this@GithubActivity
+        }
+        setContentView(binding.root)
+        initAdapter()


Binding 초기화 구문도 의미있는 함수 단위로 분리해보면 어떨까요?

아하!! 해당부분도 initBinding()등으로 함수화 시키도록 하겠습니다!! 피드백 감사합니다ㅎㅎ

wisemuji · 2023-09-01T15:01:06Z

app/src/main/java/camp/nextstep/edu/github/ui/GithubViewModel.kt

+    private val _errorEvent: MutableLiveData<Event<Unit>> = MutableLiveData()
+    val errorEvent: LiveData<Event<Unit>> = _errorEvent


이런 상황에서 SingleLiveData와 같은 것을 활용하기도 합니다. 꼭 반영하지 않고 참고만 해주셔도 좋습니다 🙂

https://medium.com/prnd/mvvm%EC%9D%98-viewmodel%EC%97%90%EC%84%9C-%EC%9D%B4%EB%B2%A4%ED%8A%B8%EB%A5%BC-%EC%B2%98%EB%A6%AC%ED%95%98%EB%8A%94-%EB%B0%A9%EB%B2%95-6%EA%B0%80%EC%A7%80-31bb183a88ce

그러네요!! SingleLiveData로 할 수 있겠네요!!
이전 강의에서 Event로 래핑하는걸로 했어서 유지하려고 했었는데 해당 방법으로도 수정하도록 하겠습니다~ 감사합니다!!

wisemuji · 2023-09-01T15:04:39Z

app/src/main/java/camp/nextstep/edu/github/ui/RepositoryAdapter.kt

+
+        fun bind(item: Repository) {
+            binding.model = item
+            binding.executePendingBindings()


executePendingBindings을 통해 바로 바인딩 변경사항을 반영하도록 명령할 수 있지만, 지금은 꼭 필요한 상황이라고 보이지는 않네요. 성능상 이점을 가져가기 위해서는 불필요한 상황에서는 제거하는게 좋습니다.

생각해보니 그러네요!! 해당 부분 executePendingBIndings() 제거하도록 하겠습니다!!

wisemuji · 2023-09-01T15:05:47Z

app/src/main/res/layout/activity_github.xml

+            app:layout_constraintBottom_toBottomOf="parent"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintEnd_toEndOf="parent"
+            tools:listitem="@layout/item_repository"


다음 명령어를 사용하여 코드를 포맷해주시면 어떨까요?

Ctrl + Alt + L (Windows/Linux)

Option + Cmd + L (Mac)

정렬화가 안됐었군요ㅠㅠ 해당 부분 수정하도록 하겠습니다!! 감사합니다~

wisemuji · 2023-09-01T15:06:45Z

app/src/main/res/layout/item_repository.xml

+        <TextView
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:textAppearance="@style/TextAppearance.MaterialComponents.Headline6"
+            tools:text="@{model.fullName}" />
+
+        <TextView
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginTop="4dp"
+            android:singleLine="true"
+            android:textAppearance="@style/TextAppearance.MaterialComponents.Body2"
+            tools:text="@{model.description}" />


tools:text가 아니라 android:text로 값을 세팅해줘야 하지 않을까요?

아!! 맞네요ㅠㅠ 해당 부분 수정하도록 하겠습니다!!

wisemuji · 2023-09-01T15:11:04Z

domain/src/main/java/com/nextstep/edu/domain/usecase/GetRepositoryUseCase.kt

+    private val repository: RemoteRepository
+) {
+    suspend operator fun invoke(): Result<List<Repository>> {
+        return runCatching { repository.getRepositories() }


runCatching으로 감싸 응답을 Result 타입으로 받도록 해주셨네요 👍

기존 플젝할때도 네트워크 실패나 에러 캐치를 위해서 Result객체로 매핑해서 작업하도록 했었긴 했는데, Result객체로 매핑시 가독성 측면에서도 좋아지는 장점이 있더라구요😄

wisemuji · 2023-09-01T15:12:48Z

data/src/main/java/com/nextstep/edu/data/di/NetworkModule.kt

-    fun provideRetrofit(): Retrofit = Retrofit.Builder()
+@Module
+@InstallIn(SingletonComponent::class)
+internal class NetworkModule {


Hilt도 잘 적용해주셨습니다 👍
이전 단계에서 수동 DI 구조를 잘 잡아두셔서 Hilt 적용도 수월하셨을 것 같아요.

수동 DI 미리 해놓고 하니까 훨씬 수월하긴 하더라구요!!

Gyuil-Hwnag · 2023-09-01T17:54:15Z

@wisemuji
리뷰어님 피드백 주신 내용 정말 감사합니다🙇‍♂️
해당 내용 수정 해서 요청했습니다~
마지막 미션 정말 고생 많으셨습니다!!

작업 내용

Binding 부분 함수화 [4e7ba7c] [e8741f5]
Event 래핑 -> SingleLiveEvent 수정 (해당 내용의 경우 이 링크를 참고 했습니다!) [34f7e88]

// SingleLiveEvent.kt
class SingleLiveEvent<T> : MutableLiveData<T>() {
  ...
}

// 사용 예시
private val _errorEvent: SingleLiveEvent<Unit> = SingleLiveEvent()
val errorEvent: LiveData<Unit> = _errorEvent

binding.executePendingBindings() 제거 (변화가 없는 Adapter일 경우 성능상 제거하는 게 나음) [b71eef7]
xml 코드 정렬 [3643a0c]
tool:text -> android:text [685ba27]

wisemuji

마지막 미션 넘 고생 많으셨습니다 규일님! 👏

마지막까지 리뷰 반영 정말 잘 해주셨어요.
이 미션은 이만 마무리하도록 하겠습니다.

앞으로도 응원하겠습니다 💪

Gyuil-Hwnag added 26 commits August 31, 2023 22:24

fix. step2 1차 피드백 반영(Mapper 함수 최상단으로 끌어올리기)

3587ff6

fix. step2 1차 피드백 반영(githubRepository.json 필요없는 부분 제거)

1dc3392

fix. step2 1차 피드백 반영(테스트 코드 함수명 수정)

f9b1364

fix. step2 1차 피드백 반영(GithubService 주석 수정)

df32b1e

fix. step2 1차 피드백 반영(GithubService 주석 수정)

e8caaf1

fix. README step2 1차 피드백 내용 반영

b00cec4

feat. gradle 라이브러리 추가

157f1f7

fix. activity & viewmodel & usecase 초기세팅

8eb2b11

fix. repository id 필드 추가

b226467

feat. gradle hilt 라이브러리 추가

3d4bd6e

feat. data & domain 모듈 hilt 세팅 추가

e2c698f

feat. repositoryAdapter 추가

82d250e

feat. activiy adapter 연결 및 viewModel getRepositoryUsecase 메서드 추가

71c80b0

feat. android 인터넷 권한 추가

5bf793f

fix. git api descrption null 오류 수정

d6624a4

feat. GetRepositoryUseCase 테스트 코드 추가

768ac94

feat. GetRepositoryUseCase 테스트 코드 케이스 추가

e9b29b7

fix. app 모듈 클래스 패키지 이동

64cf054

feat. app 모듈 테스트 라이브러리 추가

ce8277c

fix. ViewModel 테스트 코드 실행을 위한 init 메서드 수정

d9c7e44

feat. ViewModel 테스트 코드 추가

898559b

feat. step3 README 내용 추가

4e06701

fix. 사용 안하는 라이브러리 제거

41797b4

fix. 사용 안하는 인스턴스 제거 및 util 패키지 이동

8c90df6

fix. gradle 라이브러리 수정

216147a

fix. gitaction 빌드 오류 수정

6fd0e9c

wisemuji requested changes Sep 1, 2023

View reviewed changes

Gyuil-Hwnag added 3 commits September 2, 2023 02:31

fix. step3 피드백 반영(Binding 부분 함수화)

4e7ba7c

fix. step3 피드백 반영(Binding 부분 함수화 네이미 수정)

e8741f5

fix. step3 피드백 반영(Event 래핑 -> SingleLiveEvent 수정)

34f7e88

Gyuil-Hwnag added 4 commits September 2, 2023 02:43

fix. step3 피드백 반영(binding.executePendingBindings() 제거)

b71eef7

fix. step3 피드백 반영(xml 코드 정렬)

3643a0c

fix. step3 피드백 반영(tool:text -> android:text)

aa0070a

fix. step3 1차 피드백 반영 내용 수정

685ba27

wisemuji approved these changes Sep 2, 2023

View reviewed changes

wisemuji merged commit 54f7270 into next-step:gyuil-hwnag Sep 2, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step3 - GitHub(UI) #106

Step3 - GitHub(UI) #106

Gyuil-Hwnag commented Aug 31, 2023 •

edited

Loading

wisemuji left a comment

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

wisemuji Sep 1, 2023

Gyuil-Hwnag Sep 1, 2023

Gyuil-Hwnag commented Sep 1, 2023

wisemuji left a comment

		private val _errorEvent: MutableLiveData<Event<Unit>> = MutableLiveData()
		val errorEvent: LiveData<Event<Unit>> = _errorEvent

Step3 - GitHub(UI) #106

Step3 - GitHub(UI) #106

Conversation

Gyuil-Hwnag commented Aug 31, 2023 • edited Loading

작업 내용

step2 1차 피드백

wisemuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gyuil-Hwnag commented Sep 1, 2023

작업 내용

wisemuji left a comment

Choose a reason for hiding this comment

Gyuil-Hwnag commented Aug 31, 2023 •

edited

Loading