Feature/retrofit by ko0chan · Pull Request #4 · iron-mental/NaverMovie

ko0chan · 2020-09-02T12:40:18Z

레트로핏 통신

error 레트로핏으로 통신을 리팩토링 하면서 Recyclerview 와 연결오류

wswon · 2020-09-02T12:43:53Z

app/src/main/java/com/example/navermovie/MainActivity.kt

+    private val clientId = "kHLvopePGjYBNBI1P4Us"
+    private val clientSecret = "GWjKNlXW_n"


고정적인 값은 상수로 사용하는게 좋아요

Suggested change

private val clientId = "kHLvopePGjYBNBI1P4Us"

private val clientSecret = "GWjKNlXW_n"

private const val CLIENT_SECRET = "GWjKNlXW_n"

수정 완료했습니다!

wswon · 2020-09-02T12:44:53Z

app/src/main/java/com/example/navermovie/MainActivity.kt

-                println("Success to execute request : $body")
+    private fun searchByKeyword(keyword: String) {
+        val retrofit = Retrofit.Builder()
+            .baseUrl("https://openapi.naver.com/")


base Url 도 상수로 관리하는게 좋아보여요

수정 완료했습니다!

wswon · 2020-09-02T12:47:43Z

app/src/main/java/com/example/navermovie/MovieSearchRequest.kt

@@ -0,0 +1,9 @@
+package com.example.navermovie
+
+data class MovieSearchRequest(


Request 보다는 Response 가 적절한 네이밍일 것 같고, (Request 할때 사용하는 값이 아님)
@SerializedName 을 붙여서 응답시 사용되는 클래스임을 명시적으로 나타내는게 좋을 것 같아요

수정하였습니다

wswon · 2020-09-02T12:48:49Z

app/src/main/java/com/example/navermovie/NaverAPI.kt

+        @Query("query") query: String,
+        @Query("display") display: Int? = null,
+        @Query("start") start: Int? = null
+        ):retrofit2.Call<MovieSearchRequest>


Suggested change

):retrofit2.Call<MovieSearchRequest>

): retrofit2.Call<MovieSearchRequest>

수정했습니다!

wswon · 2020-09-02T12:53:36Z

app/src/main/java/com/example/navermovie/RecyclerViewAdapter.kt

                val webpage = Uri.parse("${data.link}")
                val webIntent = Intent(Intent.ACTION_VIEW, webpage)
-                view.getContext().startActivity(webIntent);
+                view.context.startActivity(webIntent);


여기(ViewHolder)에서 startActivity가 되는게 아니라,
Activity로 item이 눌렸다는 사실(+ data.link) 를 전달만 하고
실제 startActivity 실행은 Activity에서 되도록 해보면 좋을 것 같아요.

Parkduksung · 2020-09-03T00:29:45Z

app/src/main/java/com/example/navermovie/MainActivity.kt

+            recyclerView.layoutManager = LinearLayoutManager(this, LinearLayout.VERTICAL, false)
+            recyclerView.setHasFixedSize(true)


recyclerView.run{} 방식으로 Scope 함수를 사용해 보시는 것도 괜찮은 방법일 것 같습니다.

Parkduksung · 2020-09-03T00:33:14Z

app/src/main/java/com/example/navermovie/RecyclerViewAdapter.kt

 import kotlinx.android.synthetic.main.movie_item_layout.view.*

-class RecyclerViewAdapter(private val homefeed: Homefeed) :
+class RecyclerViewAdapter(private val test: MovieSearchRequest) :


조심스럽게 말씀드려봅니다..생성자 형태로 외부에서 주입받는 방식 말고 다른 방법을 사용해 보시는게 어떠신지...

추후에 더 공부해보고 적용시키도록 하겠습니다!

Parkduksung · 2020-09-03T00:34:51Z

app/src/main/java/com/example/navermovie/RecyclerViewAdapter.kt

    RecyclerView.Adapter<RecyclerViewAdapter.ViewHolder>(){
    override fun getItemCount(): Int {
-        return homefeed.items.count()
+        return test.items.count()


override fun getItemCount(): Int = test.items.count() 이런식으로도 가능하답니다.

수정하였습니다!

이러한 문법 차이가 그냥 단순한 문법 차이인 것인가요 ?

음.. 제 주관적입니다만.. 더 직관적이지 않나 생각이 드네요.

Parkduksung · 2020-09-03T00:35:59Z

app/src/main/java/com/example/navermovie/RecyclerViewAdapter.kt

+        return ViewHolder(view)
    }

    class ViewHolder(private val view: View) : RecyclerView.ViewHolder(view) {


ViewHolder 클래스를 따로 분류해서 관리해 보시는 방법도 있어요!

Parkduksung · 2020-09-03T00:37:11Z

app/src/main/java/com/example/navermovie/RecyclerViewAdapter.kt

-        fun bind(data: Item){
-            Glide.with(view.context).load(data.image)
-                .apply(RequestOptions().override(300, 450))
-                .apply(RequestOptions.centerCropTransform())


이걸 빼신 이유가 무엇인가요??

음.. 꼭 빼야겠다 싶어서 뺸건 아니지만 기초단계라고 생각이 들어서 필수적인 요소인 것들만 코딩을 진행하고 싶었습니다

그냥 궁금해서 ^^

Parkduksung · 2020-09-03T00:38:28Z

app/src/main/java/com/example/navermovie/NaverAPI.kt

+        @Header("X-Naver-Client-Id") clientId: String,
+        @Header("X-Naver-Client-Secret") clientSecret: String,
+        @Query("query") query: String,
+        @Query("display") display: Int? = null,


? = null 로 하신이유가 따로 있으신가요?

검색을 할 때에 필수 요청 변수가 아니라서 검색 필터로 사용하지 않으려고 null처리를 하였습니다.

ko0chan added 6 commits September 2, 2020 21:17

레트로핏 라이브러리 추가

a271f17

add movie search request model

b128179

data class rename

8f06d00

retrofit 네이버API통신 interface 추가

9cd32c3

code style refactor

78267b2

retrofit 통신 변경

d7111d1

ko0chan added the request codeReview label Sep 2, 2020

ko0chan requested review from Parkduksung and wswon September 2, 2020 12:40

ko0chan assigned wswon, Parkduksung and ko0chan Sep 2, 2020

wswon unassigned wswon and Parkduksung Sep 2, 2020

wswon reviewed Sep 2, 2020

View reviewed changes

Parkduksung reviewed Sep 3, 2020

View reviewed changes

Parkduksung added answer codeReview and removed request codeReview labels Sep 3, 2020

		private val clientId = "kHLvopePGjYBNBI1P4Us"
		private val clientSecret = "GWjKNlXW_n"

	private val clientId = "kHLvopePGjYBNBI1P4Us"
	private val clientSecret = "GWjKNlXW_n"
	private const val CLIENT_SECRET = "GWjKNlXW_n"

		@@ -0,0 +1,9 @@
		package com.example.navermovie

		data class MovieSearchRequest(

	):retrofit2.Call<MovieSearchRequest>
	): retrofit2.Call<MovieSearchRequest>

		recyclerView.layoutManager = LinearLayoutManager(this, LinearLayout.VERTICAL, false)
		recyclerView.setHasFixedSize(true)

Conversation

ko0chan commented Sep 2, 2020

레트로핏 통신

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants