Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
# Step2(2023-08-30) #
* GitHub(HTTP)
* [x] GET, https://api.github.com/repositories
* [x] full_name, description 필드만
* [x] Repository 목록을 가져오는 기능은 data 모듈에 구현되어야 한다.
* [x] data 모듈의 구현체는 모두 internal class로 선언한다.
* [x]HTTP 요청을 통해 가져오는 구현체에 대한 테스트 코드를 작성한다.

# Step1(2023-08-29) #
* [x] 순수 코틀린 모듈인 domain 모듈을 만든다.
* [x] 순수 코틀린 모듈인 data 모듈을 만든다.
* [x] data 모듈은 domain 모듈에 의존해야 한다.
* [x] app 모듈은 domain 모듈에 의존해야 한다.

* GitHub(모듈 분리)
* [x] 순수 코틀린 모듈인 domain 모듈을 만든다.
* [x] 순수 코틀린 모듈인 data 모듈을 만든다.
* [x] data 모듈은 domain 모듈에 의존해야 한다.
* [x] app 모듈은 domain 모듈에 의존해야 한다.
* 1차 피드백
* [x] Android Studio linebreak 설정 추가
7 changes: 7 additions & 0 deletions buildSrc/src/main/kotlin/Configs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,11 @@ object Version {
const val compileSdk = 33
const val minSdk = 26
const val targetSdk = 33

const val retrofit = "2.9.0"
const val okHttp = "4.9.3"
const val coroutineTest = "1.6.2"

const val junit = "5.8.2"
const val truth = "1.1.3"
}
19 changes: 18 additions & 1 deletion data/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,21 @@ java {

dependencies {
implementation(project(":domain"))
}

// retrofit
implementation("com.squareup.retrofit2:retrofit:${Version.retrofit}")
implementation("com.squareup.retrofit2:converter-gson:${Version.retrofit}")
implementation("com.squareup.okhttp3:okhttp:${Version.okHttp}")
implementation("com.squareup.okhttp3:logging-interceptor:${Version.okHttp}")
implementation("com.squareup.okhttp3:mockwebserver:${Version.okHttp}")

implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:${Version.coroutineTest}")
testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:${Version.coroutineTest}")

testImplementation("org.junit.jupiter:junit-jupiter:${Version.junit}")
testImplementation("com.google.truth:truth:${Version.truth}")
}

tasks.test {
useJUnitPlatform()
}
5 changes: 5 additions & 0 deletions data/src/main/java/com/nextstep/edu/data/ApiClient.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.nextstep.edu.data

internal object ApiClient {
const val BASE_URL = "https://api.github.com/"
}
11 changes: 11 additions & 0 deletions data/src/main/java/com/nextstep/edu/data/GithubService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.nextstep.edu.data

import com.nextstep.edu.data.model.RepositoryResponse
import retrofit2.http.GET

internal interface GithubService {

// 유저 목록 가져오기
Copy link
Member

Choose a reason for hiding this comment

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

이 주석은 꼭 필요한 주석일까요? 간단한 주석을 남길 때에도 주석이 꼭 필요한 상황인지 고려하여 남기시는게 좋습니다. 비록 이 프로젝트는 혼자 관리하시고 계시지만 여러 사람이 contribute 하는 프로젝트라면 이렇게 작성된 주석이 관리되지 않을 가능성이 높아집니다.
클린 코드 책에서 설명하는 주석에 대한 장을 읽어보시면 좋을 것 같아요!
FYI: https://nesoy.github.io/articles/2018-01/CleanCode-Comment

Copy link
Author

Choose a reason for hiding this comment

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

다른 플젝에서 API 버전등 API 갯수가 늘어감에 따라서 메서드만으로는 이 API가 어떤 기능을 하는지에 대해서 알기 위해서 주석을 추가했었는데 올려주신 내용 참고해서 수정하도록 하겠습니다! 감사합니다~

@GET("repositories")
suspend fun getRepositories(): List<RepositoryResponse>
}
4 changes: 0 additions & 4 deletions data/src/main/java/com/nextstep/edu/data/MyClass.kt

This file was deleted.

20 changes: 20 additions & 0 deletions data/src/main/java/com/nextstep/edu/data/di/NetworkModule.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.nextstep.edu.data.di

import com.nextstep.edu.data.ApiClient
import com.nextstep.edu.data.GithubService
import okhttp3.OkHttpClient
import retrofit2.Retrofit
import retrofit2.converter.gson.GsonConverterFactory

internal object NetworkModule {
Copy link
Member

Choose a reason for hiding this comment

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

Step3에서 Hilt를 쉽게 적용할 수 있는 구조로 구현해주셨네요 👍


fun provideRetrofit(): Retrofit = Retrofit.Builder()
.baseUrl(ApiClient.BASE_URL)
.client(provideOkHttpClient())
.addConverterFactory(GsonConverterFactory.create())
.build()

private fun provideOkHttpClient(): OkHttpClient = OkHttpClient.Builder().build()

fun provideGithubService(retrofit: Retrofit): GithubService = retrofit.create(GithubService::class.java)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.nextstep.edu.data.model

import com.google.gson.annotations.SerializedName
import com.nextstep.edu.domain.model.Repository

internal data class RepositoryResponse(
@SerializedName("full_name")
val fullName: String,

@SerializedName("description")
val description: String
) {
fun toDomain(): Repository {
return Repository(fullName = this.fullName, description = this.description)
}
Comment on lines +13 to +15
Copy link
Member

Choose a reason for hiding this comment

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

여러 레이어의 모델이 존재할 때 각 레이어 간 어떻게 모델의 값을 변환할 것인가?는 개발자마다 많은 의견이 있는 영역입니다.
저는 개인적으로 이러한 mapper 로직을 최상위 함수로 두는 것을 선호하는데요, data 모델을 domain 모델로 바꾸는 기능은 모델 자체의 역할이라고 보기 어렵기 때문입니다. data 모델을 다른 모델로 변환할 일이 또 생기더라도 원본 모델에 변화를 줄 필요는 없다고 생각합니다.

참고로 PRND 컴퍼니의 안드로이드 스타일 가이드에서도 이런 방법을 택하고 있습니다.
https://github.com/PRNDcompany/android-style-guide/blob/main/Architecture.md#model-mapping-%EC%A0%95%EC%9D%98%EA%B5%AC%EC%A1%B0

주관적인 의견이니 참고만 해주세요!

Copy link
Author

@Gyuil-Hwnag Gyuil-Hwnag Aug 31, 2023

Choose a reason for hiding this comment

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

이전에는 계속 Mapper.kt를 만들고 PRND처럼 mapper로직을 최상위 함수로 뽑아두었는데, 이번에 다른 방법으로 해보려고 작성했었습니다!!
이전 처럼 Mapper.kt 에 Mapper함수를 최상단으로 올려두고 해야겠네요ㅎㅎ 감사합니다!!

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.nextstep.edu.data.repository

import com.nextstep.edu.data.GithubService
import com.nextstep.edu.data.model.RepositoryResponse
import com.nextstep.edu.domain.repository.RemoteRepository
import com.nextstep.edu.domain.model.Repository

internal class RemoteRepositoryImpl(
private val githubService: GithubService
): RemoteRepository {
override suspend fun getRepositories(): List<Repository> {
return githubService.getRepositories().map(RepositoryResponse::toDomain)
}
}
71 changes: 71 additions & 0 deletions data/src/test/java/com.nextstep.edu.data/GithubServiceTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package com.nextstep.edu.data

import com.google.common.truth.Truth.assertThat
import com.nextstep.edu.data.model.RepositoryResponse
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import retrofit2.Retrofit
import retrofit2.converter.gson.GsonConverterFactory
import java.io.File

@ExperimentalCoroutinesApi
class GithubServiceTest {

private val testDispatcher = StandardTestDispatcher()
private val testScope = TestScope(testDispatcher)

private lateinit var server: MockWebServer
private lateinit var service: GithubService

@BeforeEach
fun setup() {
server = MockWebServer()
service = Retrofit.Builder()
.addConverterFactory(GsonConverterFactory.create())
.baseUrl(server.url(""))
.build()
.create(GithubService::class.java)
}

@Test
fun `getRepositories 호출`() = testScope.runTest {
Copy link
Member

Choose a reason for hiding this comment

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

테스트명을 통해 어떤 것을 테스트하고자 하는 것이 제대로 드러나지 않는다고 생각합니다. 테스트명을 작성하실 때에는

  1. 검증하고자 하는 로직을 명확하게 정하고 (검증이 필요한 로직이 두 개 이상이면 필요에 따라 또 다른 함수로 분할한다)
  2. 그것을 함수명으로 드러내보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

아하 그러네요!! 함수명만 봤을 때 어떤 결과가 나와야 하는지 잘 유추되지 않네요
해당 내용 작성시 유의사항 1, 2 유의해서 작성하도록 하겠습니다!! 감사합니다~

// given : githubRepositories.json 파일이 주어졌을 때
val response = MockResponse()
.setBody(File("src/test/resources/githubRepositories.json").readText())
server.enqueue(response)

// when : getRepositories() 를 요청 하면
val actual = service.getRepositories()

// then : 결과의 첫번째의 full_name은 mojombo/grit 이다.
val expected = RepositoryResponse(
fullName = "mojombo/grit",
description = "**Grit is no longer maintained. Check out libgit2/rugged.** Grit gives you object oriented read/write access to Git repositories via Ruby."
)
assertThat(actual[0]).isEqualTo(expected)
}

@Test
fun `getRepositories 호출2`() = testScope.runTest {
// given : githubRepositories.json 파일이 주어졌을 때
val response = MockResponse()
.setBody(File("src/test/resources/githubRepositories.json").readText())
server.enqueue(response)

// when : getRepositories() 를 요청 하면
val actual = service.getRepositories()

// then : 결과의 두번째의 full_name은 wycats/merb-core 이다.
val expected = RepositoryResponse(
fullName = "wycats/merb-core",
description = "Merb Core: All you need. None you don't."
)
assertThat(actual[1]).isEqualTo(expected)
}
}
Loading