Skip to content
Open
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
25 changes: 25 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
plugins {
id 'com.android.application'
id 'org.jetbrains.kotlin.android'
id 'org.jetbrains.kotlin.plugin.serialization' version '1.9.0'
}

Properties properties = new Properties()
properties.load(project.rootProject.file('local.properties').newDataInputStream())

android {
namespace 'com.sopt.now'
compileSdk 34
Expand All @@ -15,6 +19,7 @@ android {
versionName "1.0"

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
buildConfigField "String", "AUTH_BASE_URL", properties["base.url"]
}

buildTypes {
Expand All @@ -30,6 +35,11 @@ android {
kotlinOptions {
jvmTarget = '17'
}
buildFeatures {
viewBinding true
dataBinding true
Comment on lines +39 to +40
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

뷰바인딩과 데이터바인딩의 차이를 아시나요?

혹시 데이터바인딩을 어디에 사용하셨나요?! 😁

buildConfig true
}
}

dependencies {
Expand All @@ -38,7 +48,22 @@ dependencies {
implementation 'androidx.appcompat:appcompat:1.6.1'
implementation 'com.google.android.material:material:1.11.0'
implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
implementation 'androidx.activity:activity:1.8.0'

implementation 'com.squareup.retrofit2:retrofit:2.9.0'
implementation 'org.jetbrains.kotlinx:kotlinx-serialization-json:1.4.1'
implementation 'com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter:1.0.0'

// define a BOM and its version
implementation(platform("com.squareup.okhttp3:okhttp-bom:4.10.0"))

// define any required OkHttp artifacts without version
implementation("com.squareup.okhttp3:okhttp")
implementation("com.squareup.okhttp3:logging-interceptor")
implementation("androidx.navigation:navigation-fragment-ktx:2.7.7")
implementation("androidx.navigation:navigation-ui-ktx:2.7.7")
Comment on lines +51 to +64
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

기능별로 주석으로 정리해주면 좋을 것 같아요~~!!

testImplementation 'junit:junit:4.13.2'
androidTestImplementation 'androidx.test.ext:junit:1.1.5'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1'

}
10 changes: 9 additions & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools">

<uses-permission android:name="android.permission.INTERNET" />
<application
android:allowBackup="true"
android:dataExtractionRules="@xml/data_extraction_rules"
Expand All @@ -11,16 +12,23 @@
android:roundIcon="@mipmap/ic_launcher_round"
android:supportsRtl="true"
android:theme="@style/Theme.NOWSOPTAndroid"
android:usesCleartextTraffic="true"
tools:targetApi="31">
<activity
android:name=".MainActivity"
android:name=".presentation.auth.login.LoginActivity"
android:exported="true">
<intent-filter>
<action android:name="android.intent.action.MAIN" />

<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
<activity
android:name=".presentation.main.MainActivity"
android:exported="false"></activity>
<activity
android:name=".presentation.auth.signup.SignupActivity"
android:exported="false"></activity>
</application>

</manifest>
15 changes: 0 additions & 15 deletions app/src/main/java/com/sopt/now/MainActivity.kt

This file was deleted.

41 changes: 41 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/ApiFactory.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.sopt.now.presentation

import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory
import com.sopt.now.BuildConfig
import kotlinx.serialization.json.Json
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.OkHttpClient
import okhttp3.logging.HttpLoggingInterceptor
import retrofit2.Retrofit
import java.util.concurrent.TimeUnit

object ApiFactory {
private const val BASE_URL: String = BuildConfig.AUTH_BASE_URL

// HTTP 로깅 인터셉터 설정
private val loggingInterceptor = HttpLoggingInterceptor().apply {
level = HttpLoggingInterceptor.Level.BODY
}

// OkHttpClient 설정
private val okHttpClient = OkHttpClient.Builder()
.addInterceptor(loggingInterceptor) // 로깅 인터셉터 추가
.connectTimeout(30, TimeUnit.SECONDS) // 연결 타임아웃
.readTimeout(30, TimeUnit.SECONDS) // 읽기 타임아웃
.writeTimeout(30, TimeUnit.SECONDS) // 쓰기 타임아웃
.build()
Comment on lines +21 to +26
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

따로 타임아웃을 설정해두신 이유가 있나요?


// Retrofit 인스턴스 생성
val retrofit: Retrofit = Retrofit.Builder()
.baseUrl(BASE_URL)
.client(okHttpClient)
.addConverterFactory(Json.asConverterFactory("application/json".toMediaType()))
.build()

inline fun <reified T> create(): T = retrofit.create(T::class.java)
}


object ServicePool {
val authService = ApiFactory.create<AuthService>()
}
24 changes: 24 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/AuthService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.sopt.now.presentation

import retrofit2.Call
import retrofit2.http.Body
import retrofit2.http.GET
import retrofit2.http.Header
import retrofit2.http.POST

interface AuthService {
@POST("member/join")
fun signUp(
@Body request: RequestSignUpDto,
): Call<ResponseSignUpDto>

@POST("member/login")
fun login(
@Body request: RequestLoginDto
): Call<ResponseLoginDto>
Comment on lines +14 to +18
Copy link
Copy Markdown
Member

@leeeyubin leeeyubin May 5, 2024

Choose a reason for hiding this comment

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

저는 함수명을 구체적으로 적는 걸 좋아해서 postLoginToServer()처럼 작성해주는 것도 좋을 것 같아요!


@GET("member/info")
fun getUserInfo(
@Header("memberid") memberId: Int
): Call<ResponseUserInfoDto>
}
12 changes: 12 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/RequestLoginDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.sopt.now.presentation

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class RequestLoginDto(
@SerialName("authenticationId")
val authenticationId: String,
@SerialName("password")
val password: String
)
16 changes: 16 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/RequestSignUpDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.sopt.now.presentation

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class RequestSignUpDto(
@SerialName("authenticationId")
val authenticationId: String,
@SerialName("password")
val password: String,
@SerialName("nickname")
val nickname: String,
@SerialName("phone")
val phone: String,
)
12 changes: 12 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/ResponseLoginDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.sopt.now.presentation

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class ResponseLoginDto(
@SerialName("code")
val code: Int,
@SerialName("message")
val message: String,
)
12 changes: 12 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/ResponseSignUpDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.sopt.now.presentation

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class ResponseSignUpDto(
@SerialName("code")
val code: Int,
@SerialName("message")
val message: String,
)
24 changes: 24 additions & 0 deletions app/src/main/java/com/sopt/now/presentation/ResponseUserInfoDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.sopt.now.presentation

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class ResponseUserInfoDto(
@SerialName("code")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

다른 dto에서도 code와 message가 공통적으로 있어서 매번 추가해줘야하는 비효율성을 위해 BaseResponse라는 Wrapper class가 있습니다! 이걸 사용하는건 개인의 취향이기 때문에 참고만 해주세용

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이 부분에 대해서 어떤 장단점이 있을지 고민해보시면 좋을 것 같아요!! 물론 제가 꿀팁공유에 올렸었지만요 🤭

val code: Int,
@SerialName("message")
val message: String,
@SerialName("data")
val data: UserData,
)

@Serializable
data class UserData(
@SerialName("authenticationId")
val authenticationId: String,
@SerialName("nickname")
val nickname: String,
@SerialName("phone")
val phone: String,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package com.sopt.now.presentation.auth.login

import android.content.Intent
import android.os.Bundle
import android.util.Log
import android.widget.Toast
import androidx.activity.viewModels
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import com.sopt.now.databinding.ActivityLoginBinding
import com.sopt.now.presentation.RequestLoginDto
import com.sopt.now.presentation.ResponseLoginDto
import com.sopt.now.presentation.ResponseUserInfoDto
import com.sopt.now.presentation.ServicePool
import com.sopt.now.presentation.auth.signup.SignupActivity
import com.sopt.now.presentation.main.MainActivity
import retrofit2.Call
import retrofit2.Callback
import retrofit2.Response

data class LoginState(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

특별한 이유가 있지 않은 이상, 한 파일에 한 클래스로만 작성해주세요 !

val isSuccess: Boolean,
val message: String,
val memberId: String? = null
)
class LoginViewModel : ViewModel() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

한 파일에 액티비티와 뷰모델이 있는데 두개 파일로 분리해주세요!

private val authService by lazy { ServicePool.authService }
val liveData = MutableLiveData<LoginState>()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

프로퍼티명은 최대한 그 용도에 맞게 네이밍 부탁드려요! ex) loginState
더불어, 해당 프로퍼티가 다른 클래스에서 접근은 하되, 수정되지 못하게끔 backing properties 해주세요

fun getUserInfo(memberId: Int): Call<ResponseUserInfoDto> {
return authService.getUserInfo(memberId)
}
fun login(request: RequestLoginDto) {
authService.login(request).enqueue(object : Callback<ResponseLoginDto> {
override fun onResponse(
call: Call<ResponseLoginDto>,
response: Response<ResponseLoginDto>
) {
if (response.isSuccessful) {
val memberId = response.headers()["location"]

liveData.value = LoginState(
isSuccess = true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LoginState가 서버통신 처리 결과에 대해서 데이터를 담아준 것 같은데, message와 memberId가 꼭 필요할까요? 또한 지금은 로직이 복잡한 앱이 아니기 때문에 단순한 서버통신만 있겠지만, 추후 좀 더 큰 프로젝트를 운영한다면 다양한 요인의 실패가 있을거에요! 그래서 불린값으로 서버통신 성공 여부를 다루는 것보다는 다양한 케이스에 맞게 상수로 관리하는게 좋을 것 같습니다 ~!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

동의합니다! LoginState가 서버통신 처리 결과를 담는 것이라면 memberId는 따로 관리해주는 게 좋을 것 같네요!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

enum class로 SUCCESS, NETWORK_ERROR 등을 추가해서 관리하면 좋을 것 같네요!

message = "로그인 성공 user의 id: $memberId ",
memberId = memberId
)
} else {
Log.e(
"LoginError",
"HTTP ${response.code()}: ${response.errorBody()?.string()}"
)
Comment on lines +48 to +51
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

로그는 지워줍시다!

handleError(response)
}
}

override fun onFailure(call: Call<ResponseLoginDto>, t: Throwable) {
Log.e("LoginError", "로그인 중 오류 발생: ${t.localizedMessage}")
liveData.value = LoginState(isSuccess = false, message = "서버 에러 발생: ${t.localizedMessage}")
}
})
}
private fun handleError(response: Response<ResponseLoginDto>) {
val message = when (response.code()) {
400 -> "잘못된 요청입니다. 입력 값을 확인하세요."
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

제가 위에서 말한 다양한 실패요인이 여기에 있네요! 입력값 오류, 이미 등록된 사용자, 그외 서버오류 등 상수로 loginState를 표현해주시면 좋을 것 같아요! 시간이 되신다면 UiState에 대해서 공부해보시는 것도 추천합니다

409 -> "이미 등록된 사용자입니다."
else -> "로그인 실패: ${response.message()}"
}
liveData.value = LoginState(isSuccess = false, message = message)
}
}
class LoginActivity : AppCompatActivity() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

로그인 액티비티랑 뷰모델 파일 분리해주세요!

private val binding by lazy { ActivityLoginBinding.inflate(layoutInflater) }
private val viewModel by viewModels<LoginViewModel>()

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(binding.root)
initViews()
initObserver()
setUserData() // user data 세팅
}

private fun initViews() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

버튼 클릭하는 부분이면 명확하게 함수 네이밍을 지어주셔야 합니다. ex) clickButtonListener()
어떤 목적의 코드인지에 따라 함수 네이밍해주는 것도 중요합니다 ! 함수명만 봤을 때 어떤 역할을 해주는 친구인지 예측이 갈 수 있도록 직관적으로 네이밍 지어주세요 ~

// 로그인 버튼 CLICK
binding.btnLogin.setOnClickListener {
viewModel.login(getLoginRequestDto())
}
// 회원가입 버튼 CLICK
binding.btnSignup.setOnClickListener {
Intent(this@LoginActivity, SignupActivity::class.java).let {
startActivity(it)
}
}
}

private fun initObserver() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

initObserver라는 함수 이름으로 이 역할을 충분히 설명 가능한가요?

viewModel.liveData.observe(this) { loginState ->
Toast.makeText(
this@LoginActivity,
loginState.message,
Toast.LENGTH_SHORT
).show()

if (loginState.isSuccess) {
val intent = Intent(this@LoginActivity, MainActivity::class.java).apply {
loginState.memberId?.let { memberId ->
putExtra("memberId", memberId)
}
}
startActivity(intent)
Comment on lines +104 to +110
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (loginState.isSuccess) {
val intent = Intent(this@LoginActivity, MainActivity::class.java).apply {
loginState.memberId?.let { memberId ->
putExtra("memberId", memberId)
}
}
startActivity(intent)
if (loginState.isSuccess) {
Intent(this@LoginActivity, MainActivity::class.java).apply {
loginState.memberId?.let { memberId ->
putExtra("memberId", memberId)
startActivity(this)
}
}

로도 가능할 것 같네요!

finish()
}
}
}

private fun setUserData() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

setUserData() 보다는 getMemeberId()가 좀 더 직관적일 것 같네요 말 그대로 memberId 데이터를 받는 부분이니까용 !

val memberId = intent.getStringExtra("userId")?.toIntOrNull() ?: 0
viewModel.getUserInfo(memberId)
}

private fun getLoginRequestDto(): RequestLoginDto {
val id = binding.etId.text.toString()
val password = binding.etPw.text.toString()
return RequestLoginDto(
authenticationId = id,
password = password,
)
}
}
Loading