Step2 Github HTTP#45
Conversation
malibinYun
left a comment
malibinYun
left a comment
There was a problem hiding this comment.
2단계 미션 고생 많으셨어요!
생각해보면 좋을 점들에 코멘트 남겨두었습니다. 피드백 반영 후 다시 리뷰 요청 주세요!
또, Retrofit Test에서 실패인 케이스 또한 테스트를 작성해보세요 :) 그리고 GitRepoImpl를 포함한 모든 객체에 단위 테스트를 작성 해주세요! 💪
| val retrofitVersion = "2.9.0" | ||
| val okhttpVersion = "4.9.3" |
There was a problem hiding this comment.
이 변수들은 buildSrc에 위치시키는 것은 어떨까요?
|
|
||
| data class GitRepoDto ( |
There was a problem hiding this comment.
Dto라는 이름보다는, 서버 요청에대한 응답 값이니, Response 정도의 이름은 어떨까요?
There was a problem hiding this comment.
또, Github의 원격 Repository 정보를 불러오는 것이니, Git보단 Github이란 이름이 더 나아보입니다 :)
| internal class GitRepoImpl { | ||
| suspend fun getRepositories() : List<GitRepoDto> { | ||
| return NetworkObject.githubApi.getRepositories() | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Github Repository에 대한 Repository interface를 만들고, data module에서는 해당 구현체를 만드는 방식으로 로직을 구성해보는 것은 어떨까요?
또, Retrofit 객체는 생성자로부터 주입 받게 만들어보는 것을 제안드립니다 :)
| suspend fun getRepositories() : List<GitRepoDto> | ||
| } No newline at end of file |
There was a problem hiding this comment.
바로 List<~>를 반환받는 것 보다, Response<T>를 활용해보는 것은 어떨까요?
네트워크 실패 시, 더 세세하게 응답을 다룰 수 있을 거예요 :)
| .setBody(File("src/test/resources/githubRepo.json").readText()) | ||
| server.enqueue(response) |
There was a problem hiding this comment.
json 리소스 파일의 이름은 매우 구체적인 것이 좋습니다.같은 요청이라도 성공했을 때, 실패했을 때 뿐만 아니라 여러가지의 분기가 더 있을 수 있기 때문이에요. api 수가 많아진다면 가독성에 유의해야 합니다.
respositories-200.json
respositories-404.json
또는, 각 path를 디렉토리로 만들어주는 방법도 있습니다.
src/test/resources/repositories/200.json
src/test/resources/repositories/404.json
2 participants
피드백 반영 및 2단계 작업하였습니다! 리뷰해주시면 감사하겠습니다!