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

Seminar4 #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Seminar4 #5

wants to merge 3 commits into from

Conversation

ssonghyun101
Copy link
Contributor

뒷북,,,,,4차과제,,,,,도착,,,,,,,,,,,

Copy link

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

생각해볼 지점만 추가적으로 보시면 될 것 같습니다

import com.google.gson.annotations.SerializedName

data class RequsetLoginData(
@SerializedName("email")

Choose a reason for hiding this comment

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

Gson에서는 변수명과 받아오는 데이터의 키가 같으면 SerializedName 안붙이셔도 됩니당~

Comment on lines +10 to +14
data class Data(
val id: Int,
val name: String,
val email: String
)

Choose a reason for hiding this comment

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

Data라는 이름이 좀더 명확했으면 좋겠어. 이건 사람의 정보인 것 같은데 Data는 이걸 포함하고도 한참 넓은 범주인 것 같아

@POST("user/login")
fun postLogin(
@Body body : RequsetLoginData
) : retrofit2.Call <ResponseLoginData>

Choose a reason for hiding this comment

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

필요없는 prefix는 지우셔도 되어요

.addConverterFactory(GsonConverterFactory.create())
.build()

val service : Service = retrofit.create(Service::class.java)

Choose a reason for hiding this comment

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

안드로이드 자체에 실제로 Service라는 컴포넌트가 있어서 헷갈리지 않게 작명 유의하면 더 좋을 것 같아요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants