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

[Week4] 4주차 필수과제 구현 #9

Merged
merged 24 commits into from
Dec 5, 2024
Merged

[Week4] 4주차 필수과제 구현 #9

merged 24 commits into from
Dec 5, 2024

Conversation

beom84
Copy link
Contributor

@beom84 beom84 commented Nov 11, 2024

Related issue 🛠

Work Description ✏️

  • 작업 내용
  • 명세를 보고 3개의 API 연동
    • 유저등록 - SignUpPage
      • 회원가입페이지 UI 변경
      • 회원가입 조건 화면에 표시
    • 로그인 - LogIn 페이지
      • username, password 받는 구조로 변경
      • 로그인 실패, 성공시에 알맞은 안내문구 제시
    • 내 취미 조회 -MY 페이지
      • 이메일을 표시했던 부분에 내 취미를 표시
    • navigation을 sideEffect를 통해 관리
    • DataStore를 이용해 token 관리

Screenshot 📸

9pr.mp4

Uncompleted Tasks 😅

  • Task1

To Reviewers 📢

이번에 2주인만큼 이것저것 공부하면서 열심히 해보았습니다.많은 관심과 질문 부탁드립니다..!

Copy link

@MinseoSONG MinseoSONG left a comment

Choose a reason for hiding this comment

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

hilt까지 넣으셨군요 ! 코드 참고해서 공부해보겠습니다 !!
수고하셨어요 🙂

Comment on lines 40 to 46
compileOptions {
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}
kotlinOptions {
jvmTarget = "1.8"
jvmTarget = "11"
}

Choose a reason for hiding this comment

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

오 여기는 왜 수정됐을까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

자바 1.8로는 돌아가지 않는 무언가가 있어서 바꿨던거 같습니다

@@ -10,5 +10,5 @@ fun Context.findActivity(): Activity {
if (context is Activity) return context
context = context.baseContext
}
throw IllegalStateException("Permissions should be called in the context of an Activity")
throw IllegalStateException("permission은 context of Activity에서 불러주세요!")

Choose a reason for hiding this comment

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

이 부분은 뭘까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이전에 Acitivty Context 를 가져오기 위해 사용했는데 안쓰니까 삭제해야겠네요!

import org.sopt.and.domain.entity.UserHobby
import org.sopt.and.domain.entity.UserToken

fun ResponseGetMyHobbyDto.toDomain() : UserHobby = UserHobby(

Choose a reason for hiding this comment

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

toDomain은 뭘까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

data layer의 값을 domain으로 보낼때 사용하고 있습니다

_loginState.value.password
)
).onFailure { exception ->
Log.d("ServerExeception", "등록 실패: ${exception.message}")

Choose a reason for hiding this comment

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

로그는 지워두는게 좋지 않을까요 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!


companion object {
private const val USER_INFO_LENGTH_MAX = 8
private const val PASSWORD_TYPE = 3

Choose a reason for hiding this comment

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

PasswordType = 3은 뭘까요 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

비밀번호 형식을 지켰는지 확인할때 사용합니다!

Copy link

@Hyobeen-Park Hyobeen-Park left a comment

Choose a reason for hiding this comment

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

서버 통신도 수고하셨습니다!

implementation(libs.androidx.hilt.navigation.compose)

//dataStore
implementation("androidx.datastore:datastore-preferences:1.0.0")

Choose a reason for hiding this comment

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

datastore 사용하셨나봐요!! 데이터를 저장하는 방법이 여러개가 있는데 datastore를 선택하신 이유가 궁금합니다

Copy link
Contributor

Choose a reason for hiding this comment

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

통일성을 유지하여 버전 카탈로그를 통해 관리해도 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

안드로이드측에서 요즘에는 sharedPreference보다는 dataStore를 권장한다해서 사용하였습니다!

Copy link

@wjdrjs00 wjdrjs00 left a comment

Choose a reason for hiding this comment

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

4주차 과제 고생 많으셨습니다!!
저는 미션구현에서 계층 분리를 그동안 해오던 방식대로 했는데, 코드리뷰를 해보니까 과연 내 방식이 맞나? 싶은 생각이 들었던거 같아요! 좀 더 명확하게 공부해야겠구나 하는 생각을 가질수 있었습니다!

Comment on lines 9 to 10
id("com.google.dagger.hilt.android")
id("kotlin-kapt")

Choose a reason for hiding this comment

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

이 부분도 alias를 활용해서 일관성있게 작성하는것도 좋을거 같아요~👍🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

추가로 alias와 id의 차이가 무엇인지 알아봐도 좋을 것 같네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alias는 버젼 카탈로그를 이용할 수 있게 해주는 함수군요.. 반드시 이용해서 일관성 지켜내겠습니다

@@ -0,0 +1,5 @@
package org.sopt.and.domain.entity

Choose a reason for hiding this comment

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

비즈니스 모델에 대한 패키징을 model과 entity 중 하나로 가져가는걸로 알고 있는데 entity로 가져간 이유가 궁금합니다! 🤔 (저도 명확하게 아는게 아니라 의견을 들어보고 싶어요!ㅎ)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이번에 클린아키텍쳐를 처음 공부하고 적용해봐서 많은 걸 알지 못합니다! ..ㅠ
제가 읽은 article애서 entity를 많이 사용하여 entity를 사용했는데 model 과 entity에는 어떤 차이가 있는지 말씀해주실수 있나요?
감사합니다!!!!
@wjdrjs00

package org.sopt.and.domain.entity

data class UserLogInInfo(
val userName : String = "",

Choose a reason for hiding this comment

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

초기값을 준 이유가 궁금합니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

viewmodel에서 사용하는 값이어서 초기값을 주지 않으면 오류가 나서 사용하였습니다!

Comment on lines +44 to +51
mutableStateOf(
UIState(
getGenreTextList(),
getMainBannerImage(),
getEditorRecommendList(),
getTop20List()
)
)

Choose a reason for hiding this comment

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

이런 방식을 사용하니 ui에서 코드가 좀 더 간결하게 표현이 가능하네요! 저도 참고해서 다음에 활용해 봐야겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이런 힘나는 칭찬..감사합니다...

import org.sopt.and.domain.repository.DataStoreRepository
import javax.inject.Inject

class DataStoreSetTokenUseCase @Inject constructor(

Choose a reason for hiding this comment

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

SetTokenUseCase는 어떤지 생각을 남겨봅니다!
DataStore를 명시하는게 뭔가 DataStore가 아닌 다른걸로 변경되는 경우 해당 usecase명명까지 수정해야하지 않을까? 생각이 들었습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

동의합니다! 유즈케이스의 경우 구현 기술에 의존적인 네이밍은 피하는 게 좋아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉! 생각해보니 분리하려고 사용한건데 이렇게되면 다시 의존적으로 될수있겠네요 감사합니다!

signInUserUseCase(request)

private suspend fun setToken(token: String) {
dataStoreSetTokenUseCase.invoke(token)

Choose a reason for hiding this comment

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

invoke는 이름없이도 호출이 가능합니다!

Suggested change
dataStoreSetTokenUseCase.invoke(token)
dataStoreSetTokenUseCase(token)

Copy link
Contributor

Choose a reason for hiding this comment

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

invoke가 무엇인지 공부해보시면 좋을 듯 합니다 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

invoke는 코틀린에서 연산자라고 불리는 이름없이 �호출가능한 함수군요.. 메모해두겠습니다 감사합니다!!

Comment on lines +66 to +79
private fun isPasswordValid(): Boolean {
val password = _signupState.value.password

if (password.length < USER_INFO_LENGTH_MAX) {
var count = 0
if (password.contains(UPPER_CASE_REGEX.toRegex())) count++
if (password.contains(LOWER_CASE_REGEX.toRegex())) count++
if (password.contains(NUMBER_REGEX.toRegex())) count++
if (password.contains(SPECIAL_CHAR_REGEX.toRegex())) count++

if (count >= PASSWORD_TYPE) return true
}
return false
}

Choose a reason for hiding this comment

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

저도 저번 미션에서 받은 리뷰 사항이긴 한데 아이디, 비밀번호 검증과 같은 기능을 viewmodel에서 책임을 가져야 하나?? 생각해봤을때 아닌거 같아서 viewmodel에서 분리시키는 작업을 했습니다! 한번 분리해보는것도 좋을거 같아요! (전 어디서 가져야하는 책임이지?를 좀 생각하면서 분리했던거 같습니다!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오...저도 한번 고민하고 코드에 적용해보겠습니다

build.gradle.kts Outdated
@@ -3,4 +3,6 @@ plugins {
alias(libs.plugins.android.application) apply false
alias(libs.plugins.kotlin.android) apply false
alias(libs.plugins.kotlin.compose) apply false
id("com.google.dagger.hilt.android") version "2.51.1" apply false

Choose a reason for hiding this comment

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

여기도 alias ㅎㅎ,,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

감사하빈당ㅎㅎ

Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 열심히 공부하신 게 코드에서도 티가 나네요!
그럼 합동 세미나도 파이띵!

Comment on lines 9 to 10
id("com.google.dagger.hilt.android")
id("kotlin-kapt")
Copy link
Contributor

Choose a reason for hiding this comment

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

추가로 alias와 id의 차이가 무엇인지 알아봐도 좋을 것 같네요!

implementation(libs.androidx.hilt.navigation.compose)

//dataStore
implementation("androidx.datastore:datastore-preferences:1.0.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

통일성을 유지하여 버전 카탈로그를 통해 관리해도 좋을 것 같아요!

Comment on lines 8 to 10
override fun onCreate() {
super.onCreate()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

지워주셔도 될 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

import kotlinx.serialization.Serializable

@Serializable
data class ResponseGetMyHobbyDto(
Copy link
Contributor

Choose a reason for hiding this comment

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

BaseResponse를 사용하는 방법도 고려해보셔도 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

불필요한 코드중복을 막을 수 있을 것 같아요 바로 적용 들어가겠습니다

Comment on lines 18 to 23
@Serializable
data class ResponseSignInFailureDto(
@SerialName("code")
val code: String
)

Copy link
Contributor

Choose a reason for hiding this comment

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

실패했을 때의 DTO가 모두 동일하니 하나만 사용해도 괜찮을 것 같습니당

private val genreTextList = listOf(
private fun getGenreTextList() = listOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

val을 사용한 것과 fun을 사용한 것의 이점은 뭘까용?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

val 을 이용해 선언할시에 정적이고 바꾸지 못한다는 단점이자 장점이 있습니다
fun 의 경우 호출마다 다른 값을 반환할 수 있기에 상황에따라 값을 다르게 받을 수 있다는 이점이 있습니다

Comment on lines 67 to 69
fun checkLoginData(

) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 요상한 줄바꿈은 뭐냐능,,

signInUserUseCase(request)

private suspend fun setToken(token: String) {
dataStoreSetTokenUseCase.invoke(token)
Copy link
Contributor

Choose a reason for hiding this comment

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

invoke가 무엇인지 공부해보시면 좋을 듯 합니다 !

Comment on lines 29 to 45
init {
viewModelScope.launch {
val savedToken = getToken().first()
_token.value = UserToken(savedToken)
getUserHobby(
_token.value.token
).onFailure { exception ->
Log.d("ServerExeception", "취미 조회 실패: ${exception.message}")
}.onSuccess { response ->
_profileStatus.update {
it.copy(
hobby = response.hobby
)
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

init은 어떻게 동작할까요?
어떠한 로직을 담는 것이 좋을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

�property -> 주생성자 > init 순서로 초기화되는군요...
여기서는 필수적인 초기화 logic만 실행하고 데이터를 불러오는 비동기작업은 다른곳에서 실행하는것이 좋을 것 같습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

야무지다!!!! 짱!!!!

Comment on lines 123 to 140
companion object {
private const val USER_INFO_LENGTH_MAX = 8
private const val PASSWORD_TYPE = 3

const val UPPER_CASE_REGEX = "[A-Z]"
const val LOWER_CASE_REGEX = "[a-z]"
const val NUMBER_REGEX = "[0-9]"
const val SPECIAL_CHAR_REGEX = "[!@#\$%^&*(),.?\":{}|<>]"

val EXTRA_SIGNUP_IMAGE_LIST = listOf(
Icons.Default.CheckCircle,
Icons.Default.CheckCircle,
Icons.Default.CheckCircle,
Icons.Default.CheckCircle,
Icons.Default.CheckCircle
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

이 viewModel에서만 사용된다면 private로 정의해도 좋을 것 같아요!

Copy link

@Hyobeen-Park Hyobeen-Park left a comment

Choose a reason for hiding this comment

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

수고하셨습니당!!

Comment on lines 30 to 31


Choose a reason for hiding this comment

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

빈줄 2줄!!

val profileViewModel: MyProfileViewModel = hiltViewModel()
val profileState by profileViewModel.profileState.collectAsStateWithLifecycle()

LaunchedEffect(Unit) {

Choose a reason for hiding this comment

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

LaunchedEffect 키값으로 Unit을 사용하셨네요aunchedEffect 키값으로 Unit을 사용하셨네요 Unit을 넣는건 많이 못본 것 같은데 이유가 궁금합니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

static한 값을 넣기만 하면 상관없다고 생각해서 unit을 넣어줬습니다!

Comment on lines 39 to 40
).onFailure {
}.onSuccess { response ->

Choose a reason for hiding this comment

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

onFailure 내부가 비어있는데 꼭 필요한 부분일까요...??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

없애버리겠습니다..!

Comment on lines 105 to 112
registerUser(
RequestUserInfoRegisterDto(
_signupState.value.username,
_signupState.value.password,
_signupState.value.hobby
)
).onFailure {
}.onSuccess {

Choose a reason for hiding this comment

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

이게 dataCheck라는 함수 안에 들어있는데 함수의 이름대로라면 데이터를 확인하는 로직만 들어있어야 하는게 아닌가? 싶은 생각이 들었어요!! 이 부분은 dataCheck보다는 signUp의 역할을 하는 부분이라 분리하는 것도 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

너무 좋은 생각인거 같습니다! 용도를 생각하고 분리와 네이밍을 신중하게 해볼게요

@beom84 beom84 merged commit 5c9c3a5 into develop Dec 5, 2024
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.

[FEAT] 4주차 필수 과제
5 participants