[FEAT] WEEK4 - xml 필수과제 by nagaeng · Pull Request #8 · NOW-SOPT-ANDROID/nakyung-lee

nagaeng · 2024-05-02T21:06:00Z

📗 WORK DESCRIPTION

TODO

회원가입 API 서버 통신을 통해 성공 시 토스트 띄우기
API 명세서에 맞게 로그인 성공 시 memberId(헤더)를 포함하는 토스트 띄우기
로그인 API 서버 통신을 통해 다른 기기에서도 로그인이 가능하도록 구현
로그인 성공시 POST를 사용해서 memberId를 통한 userinfo 불러와서 마이페이지 구현

📗VIDEO (RUN)

4week-xml-essential.mp4

📗TO REVIEWER

우와
과제하다가
해가 떴어요 ..!

[feat/#1] week1 - xml / essential(필수)

amourxyoung

안녕하세요 나경님 ! 앞으로 코드리뷰를 맡게 된 김지영입니다 반갑습니다 :)
코드리뷰 반영하다가 이해가 안되거나 모르시는 부분 있으면 언제든지 연락 주세요 !
고생하셨습니다 😄😄

amourxyoung · 2024-05-04T16:59:20Z

app/src/main/java/com/sopt/now/presentation/ResponseUserInfoDto.kt

+
+@Serializable
+data class ResponseUserInfoDto(
+    @SerialName("code")


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

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

amourxyoung · 2024-05-04T16:59:24Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+import retrofit2.Callback
+import retrofit2.Response
+
+data class LoginState(


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

amourxyoung · 2024-05-04T16:59:27Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+    val message: String,
+    val memberId: String? = null
+)
+class LoginViewModel : ViewModel() {


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

amourxyoung · 2024-05-04T16:59:29Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+)
+class LoginViewModel : ViewModel() {
+    private val authService by lazy { ServicePool.authService }
+    val liveData = MutableLiveData<LoginState>()


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

amourxyoung · 2024-05-04T16:59:33Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+                    val memberId = response.headers()["location"]
+
+                    liveData.value = LoginState(
+                        isSuccess = true,


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

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

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

amourxyoung · 2024-05-04T17:00:22Z

app/src/main/java/com/sopt/now/presentation/auth/signup/SignupActivity.kt

+    val message: String,
+)
+
+class SignUpViewModel : ViewModel() {


LoginAcitivty.kt에 달았던 모든 코멘트와 동일합니다

amourxyoung · 2024-05-04T17:00:31Z

app/src/main/java/com/sopt/now/presentation/main/home/FriendListAdapter.kt

+    override fun getItemCount(): Int {
+        return friendList.size + 1 // 내 프로필을 포함하기 위해 +1을 함
+    }


Suggested change

override fun getItemCount(): Int {

return friendList.size + 1 // 내 프로필을 포함하기 위해 +1을 함

}

override fun getItemCount(): Int = friendList.size + 1

단순한 값을 반환해줄 때는 return 생략 가능합니다!

amourxyoung · 2024-05-04T17:00:35Z

app/src/main/java/com/sopt/now/presentation/main/home/HomeFragment.kt

+            ),
+            Friend(
+                profileImage = R.drawable.ic_person_black_24,
+                name = "이정하",


뭘 좀 아시는군요,,,

amourxyoung · 2024-05-04T17:00:40Z

app/src/main/java/com/sopt/now/presentation/main/home/HomeFragment.kt

+        friendListAdapter.setFriendList(mockFriendList)
+    }
+    override fun onDestroyView() {
+        super.onDestroyView()


메모리 누수 방지를 위해 friendListAdapter도 null 처리해주시는게 좋아욥

amourxyoung · 2024-05-04T17:00:49Z

app/src/main/res/layout/activity_signup.xml

+        android:layout_marginStart="70dp"
+        android:layout_marginEnd="70dp"
+        android:hint="ID를 입력하세요"
+        android:id="@+id/et_id"


xml id명은 사진처럼 네이밍해주시는게 좋습니다! ex) edt_signup_id

leeeyubin

나경이 첫 서버통신 너무너무 수고많았어요!!! 이대로 쭉 가보자~!!!

leeeyubin · 2024-05-05T04:51:57Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+                    Log.e(
+                        "LoginError",
+                        "HTTP ${response.code()}: ${response.errorBody()?.string()}"
+                    )


로그는 지워줍시다!

leeeyubin · 2024-05-05T04:54:20Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+                    val memberId = response.headers()["location"]
+
+                    liveData.value = LoginState(
+                        isSuccess = true,


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

leeeyubin · 2024-05-05T04:55:36Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

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


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)

}

}

로도 가능할 것 같네요!

leeeyubin · 2024-05-05T05:05:18Z

app/src/main/java/com/sopt/now/presentation/main/mypage/MyPageFragment.kt

+
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+
+        memberId = activity?.intent?.getStringExtra("memberId") ?: "0"
+
+        // 회원 정보를 가져오는 API 호출
+        memberId?.let { memberId ->
+            ServicePool.authService.getUserInfo(memberId.toInt()).enqueue(object : Callback<ResponseUserInfoDto> {


함수화 해주세욤,,ㅜㅜ

leeeyubin · 2024-05-05T05:08:00Z

app/src/main/res/layout/activity_login.xml

+        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintTop_toTopOf="parent"
+        />


윈도우라면 Ctrl + Alt + L로 코드 최적화 적용해주세요!

leeeyubin · 2024-05-05T05:09:00Z

app/src/main/java/com/sopt/now/presentation/AuthService.kt

+
+    @POST("member/login")
+    fun login(
+        @Body request: RequestLoginDto
+    ): Call<ResponseLoginDto>


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

MoonsuKang

안드로이드의 꽃인 API 통신 고생하셨습니다ㅎㅎ 나날이 실력이 느는 게 잘 보이네요! 화이팅~

MoonsuKang · 2024-05-05T09:24:26Z

app/src/main/java/com/sopt/now/presentation/main/mypage/MyPageFragment.kt

+
+        memberId = activity?.intent?.getStringExtra("memberId") ?: "0"
+
+        // 회원 정보를 가져오는 API 호출


UDA원칙에 따라 상태관리를 중앙화 시키는게 좋아보입니다ㅎ
상태 변경, 데이터 흐름을 ViewModel이 관리 -> Fragment는 관찰 + 반영하는 로직

MoonsuKang · 2024-05-05T09:38:12Z

app/src/main/java/com/sopt/now/presentation/main/mypage/MyPageFragment.kt

+    companion object {
+        fun newInstance(memberId: String?): MyPageFragment {
+            val fragment = MyPageFragment()
+            val args = Bundle().apply {
+                putString("memberId", memberId)
+            }
+            fragment.arguments = args
+            return fragment
+        }
+    }
+
+    override fun onCreateView(
+        inflater: LayoutInflater,
+        container: ViewGroup?,
+        savedInstanceState: Bundle?
+    ): View {
+        _binding = FragmentMypageBinding.inflate(inflater, container, false)
+        return binding.root
+    }
+
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+
+        memberId = activity?.intent?.getStringExtra("memberId") ?: "0"


newInstance()에서 memberId를 fragment의 arguments에 저장하는 로직 좋습니다!
그런데 onViewCreated에서 memberId를 activity intent로 직접 가져오고 있는데, newInstance를 통해 설정된 arguments를 사용하지 않고, activity에서 직접 데이터를 가져오면 어떤 문제가 생길 수 있을까요??

MoonsuKang · 2024-05-05T09:42:21Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+                    val memberId = response.headers()["location"]
+
+                    liveData.value = LoginState(
+                        isSuccess = true,


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

2hyunjinn · 2024-05-05T13:17:18Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+        liveData.value = LoginState(isSuccess = false, message = message)
+    }
+}
+class LoginActivity : AppCompatActivity() {


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

2hyunjinn · 2024-05-05T13:23:08Z

app/src/main/java/com/sopt/now/presentation/auth/signup/SignupActivity.kt

+    val liveData = MutableLiveData<SignUpState>()
+
+    fun signUp(signUpRequest: RequestSignUpDto) {
+        authService.signUp(signUpRequest).enqueue(object : Callback<ResponseSignUpDto> {


viewmodelScope를 사용해서 코루틴을 사용하는 것도 좋을 것 같아요!

2hyunjinn · 2024-05-05T13:23:27Z

app/src/main/java/com/sopt/now/presentation/main/MainActivity.kt

+        binding = ActivityMainBinding.inflate(layoutInflater)
+        setContentView(binding.root)
+
+        val memberId = intent.getStringExtra("memberId") // null일 경우에는 0으로 처리


코드와 관련없는 주석인 것 같습니다

junseo511

이번주 과제도 고생 많으셨습니다~~~!!!! 제 합세조 구경왔어요 🤭

junseo511 · 2024-05-05T15:30:32Z

app/build.gradle

+        viewBinding true
+        dataBinding true


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

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

junseo511 · 2024-05-05T15:30:48Z

app/build.gradle

+    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")


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

junseo511 · 2024-05-05T15:31:19Z

app/src/main/java/com/sopt/now/presentation/ApiFactory.kt

+    private val okHttpClient = OkHttpClient.Builder()
+        .addInterceptor(loggingInterceptor)  // 로깅 인터셉터 추가
+        .connectTimeout(30, TimeUnit.SECONDS) // 연결 타임아웃
+        .readTimeout(30, TimeUnit.SECONDS)    // 읽기 타임아웃
+        .writeTimeout(30, TimeUnit.SECONDS)   // 쓰기 타임아웃
+        .build()


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

junseo511 · 2024-05-05T15:32:04Z

app/src/main/java/com/sopt/now/presentation/ResponseUserInfoDto.kt

+
+@Serializable
+data class ResponseUserInfoDto(
+    @SerialName("code")


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

junseo511 · 2024-05-05T15:34:00Z

app/src/main/java/com/sopt/now/presentation/auth/login/LoginActivity.kt

+        }
+    }
+
+    private fun initObserver() {


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

junseo511 · 2024-05-05T15:34:35Z

app/src/main/java/com/sopt/now/presentation/auth/signup/SignupActivity.kt

+
+class SignUpViewModel : ViewModel() {
+    private val authService by lazy { ServicePool.authService }
+    val liveData = MutableLiveData<SignUpState>()


liveData 라는 변수명은 이 변수가 하는 일을 설명해주지 못할 것 같습니다!!

junseo511 · 2024-05-05T15:35:26Z

app/src/main/java/com/sopt/now/presentation/auth/signup/SignupActivity.kt

+    private fun handleError(response: Response<ResponseSignUpDto>) {
+        val message = when (response.code()) {
+            400 -> "잘못된 요청입니다. 입력 값을 확인하세요."
+            409 -> "이미 등록된 사용자입니다."
+            else -> "회원가입 실패: ${response.message()}"
+        }
+        liveData.value = SignUpState(isSuccess = false, message = message)
+    }


이런 방식으로 핸들링하는 것보다 더 좋은 방법이 있을 것 같아요!!

여기서 다른 분들이 달아놓은 리뷰에서 찾아보시길...!

junseo511 · 2024-05-05T15:36:48Z

app/src/main/java/com/sopt/now/presentation/main/home/HomeFragment.kt

+            description = "Last Chance - 소수빈"
+        )
+
+        val mockFriendList = listOf<Friend>(


이제 가짜 데이터는 날리고 API를 가져와보죠,,,!

junseo511 · 2024-05-05T15:37:18Z

app/src/main/java/com/sopt/now/presentation/main/home/HomeFragment.kt

+        super.onDestroyView()
+        _binding = null


Suggested change

super.onDestroyView()

_binding = null

_binding = null

super.onDestroyView()

이친구는 이렇게 사용하는게 더 안정적이랍니다?

nagaeng and others added 13 commits April 4, 2024 22:13

[FEAT/#1] 1주차 xml (로그인 및 회원가입 화면 구현)

401b208

[FEAT/#1] 1주차 xml 수정

e7b4b13

Merge pull request #2 from NOW-SOPT-ANDROID/feat/#1-week1-essential

2343d82

[feat/#1] week1 - xml / essential(필수)

[feat/#3] week2 - xml / essential(필수) : 홈화면 구현 (마이페이지 미완성)

81e878f

[feat/#4] week2 - xml / essential(필수) : 홈화면 구현 (마이페이지 intent 정보 띄우기)

0762bc2

[feat/#5] week2 - xml / essential(필수) 코리반영

f9864a1

[feat/#7] 대략적인 폴더링

0a91abb

[feat/#8] 회원가입 API 통신 재구현

ccccb63

[feat/#9] 로그인 API 구현 (memberid toast 메시지로 띄우기까지)

1fafb5d

[feat/#10] gradle 의존성 추가

4e82406

[feat/#11] memberid를 통한 마이페이지 intent 전달

0c2d40f

[feat/#12] memberId 전달 오류 수정, 마이페이지 완성

0d68e24

[fix/#13] 회원가입 성공 toast 오류 해결

57a50ab

nagaeng requested review from 2hyunjinn, MoonsuKang, kez-lab and leeeyubin May 3, 2024 06:28

nagaeng changed the title ~~Feat/#5 week4 xml essential~~ [FEAT] WEEK4 - xml 필수과제 May 3, 2024

amourxyoung reviewed May 4, 2024

View reviewed changes

leeeyubin reviewed May 5, 2024

View reviewed changes

MoonsuKang reviewed May 5, 2024

View reviewed changes

2hyunjinn reviewed May 5, 2024

View reviewed changes

2hyunjinn approved these changes May 5, 2024

View reviewed changes

junseo511 reviewed May 5, 2024

View reviewed changes


		memberId = activity?.intent?.getStringExtra("memberId") ?: "0"

		// 회원 정보를 가져오는 API 호출

Conversation

nagaeng commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📗 WORK DESCRIPTION

TODO

📗VIDEO (RUN)

📗TO REVIEWER

Uh oh!

amourxyoung left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

leeeyubin left a comment

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!

leeeyubin May 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoonsuKang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoonsuKang May 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

junseo511 left a comment

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

nagaeng commented May 2, 2024 •

edited

Loading

amourxyoung left a comment •

edited

Loading

leeeyubin May 5, 2024 •

edited

Loading

MoonsuKang May 5, 2024 •

edited

Loading