Week6 assignment by tunaunnie · Pull Request #10 · AND-SOPT-ANDROID/jiwon-yang

tunaunnie · 2024-12-06T09:18:34Z

Related issue 🛠

closed Week6 #9

Work Description ✏️

Clean Architecture 적용
- Data: datalocal, dataremote 분리하고 각각 impl 구현
- UserRepository 인터페이스 생성 및 usecase 연결

Screenshot 📸

week4와 동일

Uncompleted Tasks 😅

Task1

To Reviewers 📢

프로젝트에 전체적인 아키텍처를 적용하는 일이 많이 어려워서 워크북 내 데이트로드 소스코드와 민석님 코드를 정말 많이 참고했습니다.. 특히 의존성 주입 (di) 부분은 이해하기가 어려워서 계속 코드를 참고하다 보니 거의 구조를 똑같이 만든 것 같아요😭 Usecase와 Repository 구현체를 분리해서 의존성을 최대한 제거한다는 아이디어는 어느 정도 감이 잡힌 것 같은데, 아직 Hilt에 대한 이해는 많이 부족한 것 같습니다ㅜㅜ

tunaunnie · 2024-12-06T09:21:04Z

이전 주차 (week4) 과제까지 이번에 한 번에 진행해서 머지가 안 된 상태라 이번에도 파일 트래킹이 다 섞여버려서 코드 리뷰 진행하시기가 어려우실 것 같아요..ㅠㅠ 대략적으로 data 패키지의 datalocal, dataremote, di 패키지, domain/repository 패키지의 UserRepository, usecase 패키지 하의 파일들 확인해주시면 될 것 같습니다..!

chattymin

6주차 과제 수고하셨습니다!!
clean architecture에 대해서 한번 더 공부해보시고 리펙토링 진행해보시면 좋을 것 같아요!

새로운 기술을 배워서 쓰는 것도 좋지만, 기존의 기술을 잘 알고 사용하는게 정말정말 중요하다고 생각해요. 앱잼이 다가오고 있는데 앱잼 전에 지금까지 배운 것들을 한번 복습하는 시간을 가져본다면 앱잼때 정말 큰 도움이 될거에요 :)

app/src/main/java/org/sopt/and/di/NetworkModule.kt

chattymin · 2024-12-06T16:29:33Z

app/src/main/java/org/sopt/and/domain/repository/UserRepository.kt

+import org.sopt.and.data.dataremote.model.request.RequestCreateUserDto
+import org.sopt.and.data.dataremote.model.request.RequestGetUserDto
+import org.sopt.and.data.dataremote.model.response.ResponseCreateUserSuccessDto
+import org.sopt.and.data.dataremote.model.response.ResponseGetUserDto
+import org.sopt.and.data.dataremote.model.response.ResponseGetUserHobbyDto
+import retrofit2.Response


domain인데 data layer에 의존성을 가지고 있어요.
그리고 retrofit에 대한 의존성도 가지고 있어요.

domain모듈은 kotlin/java에 대한 의존성만 가지고 있어야 하기 때문에 의존성 제거바랍니다!

의존성 분리라는 원래 목적을 생각하면서 리팩토링해보니, 클린 아키텍처가 지향하는 구조에 대해서 조금 더 감이 잡히는 것 같아요. 리뷰 감사합니다!!

t1nm1ksun

6,7주차 내용 저는 상당히 어렵던데 고생많으셨습니다!
이제 곧 앱잼하실 텐데 화이팅입니다!!

app/src/main/java/org/sopt/and/data/dataremote/network/ApiFactory.kt

t1nm1ksun · 2024-12-08T09:09:34Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginScreen.kt

 data class LoginScreen(
-    val emailText: String,
+    val userNameText: String,
    val passwordText: String
 )


아래에 있는 로그인화면과 네이밍이 똑같아서 헷갈릴 수 있을거같아요!
그리고 만약 사용하는곳이나 다른 data class들이 많아진다면 따로 폴더를 생성해서 관리하는것도 좋습니다!

t1nm1ksun · 2024-12-08T09:17:16Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginViewModel.kt

+) : ViewModel() {
+
+    //로그인 요청을 서버로 보내기 위함
+    private val userService by lazy { ServicePool.userService }


UseCase를 통해 서버통신을 만들어 두었으니 이제 해당 코드는 사용되지 않을 것 같아요!

t1nm1ksun · 2024-12-08T09:21:37Z

app/src/main/java/org/sopt/and/presentation/signupScreen/SignUpScreen.kt

+fun StringInputValidCheck(newString: String): Boolean {
    var isValid = false
-    val inputStr : CharSequence = email
-    val pattern = Patterns.EMAIL_ADDRESS
-    val matcher = pattern.matcher(inputStr)
-    if(matcher.matches()){
-        isValid = true
+    val inputStr : CharSequence = newString
+
+    if(inputStr.length >= 8){
+        return isValid
+    } else {
+        return !isValid
    }
-    return isValid
 }

 fun PasswordValidCheck(password: String): Boolean {
-    val pattern = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)|(?=.*[a-z])(?=.*[A-Z])(?=.*[!@#\$%^&*])|(?=.*[a-z])(?=.*\\d)(?=.*[!@#\$%^&*])|(?=.*[A-Z])(?=.*\\d)(?=.*[!@#\$%^&*]).{8,20}\$".toRegex()
+    val pattern = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)|(?=.*[a-z])(?=.*[A-Z])(?=.*[!@#\$%^&*])|(?=.*[a-z])(?=.*\\d)(?=.*[!@#\$%^&*])|(?=.*[A-Z])(?=.*\\d)(?=.*[!@#\$%^&*]).{1,8}\$".toRegex()
    return password.matches(pattern)
-
 }


해당 로직들은 비지니스 로직이라고 생각합니다!
어디에 위치하면 좋을까요?

validateUserInputUseCase에 유효성 검증 로직을 옮겨서 domain 레이어로 분리했습니다!! 꼼꼼한 리뷰 감사합니다 🥲

angryPodo · 2024-12-08T13:51:02Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginViewModel.kt

+) : ViewModel() {
+
+    //로그인 요청을 서버로 보내기 위함
+    private val userService by lazy { ServicePool.userService }


DI를 사용하고 있는데 ServicePool을 직접 참조하는 것은 DI 원칙에 위배됩니다!
민석이 코멘트 처럼 사용하지 않는 코드니 제거 해도 좋을 것 같네요

angryPodo · 2024-12-08T13:52:53Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginViewModel.kt

+    private val _loginResult = MutableStateFlow<Boolean?>(null)
+    val loginResult = _loginResult.asStateFlow()


Suggested change

private val _loginResult = MutableStateFlow<Boolean?>(null)

val loginResult = _loginResult.asStateFlow()

sealed class LoginUiState {

object Initial : LoginUiState()

object Success : LoginUiState()

data class Error(val message: String) : LoginUiState()

}

private val _loginState = MutableStateFlow<LoginUiState>(LoginUiState.Initial)

val loginState = _loginState.asStateFlow()

Boolean? 대신 sealed class를 사용하여 상태를 더 명확하게 표현할 수 있어요!

angryPodo · 2024-12-08T13:54:44Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginViewModel.kt

+        _shouldShowPassword.value = !_shouldShowPassword.value
+    }
+
+    // 입력값이 조건에 맞는지 확인 (둘 다 여덟 자 이하?)


유효성 검사 로직을 별도의 UseCase나 다른 클래스로 분리하는게 좋아보여요

angryPodo · 2024-12-08T13:58:37Z

app/src/main/java/org/sopt/and/presentation/mypageScreen/MypageViewModel.kt

+            _user.value.name = userName
+            _user.value.hobby = hobby
+            _user.value.accessToken = accessToken


Suggested change

_user.value.name = userName

_user.value.hobby = hobby

_user.value.accessToken = accessToken

_user.value = _user.value.copy(

name = userName,

hobby = hobby,

accessToken = accessToken

)

직접 필드 수정은 권장되지 않는걸로 알아서 요렇게 해보는건 어때요?

angryPodo · 2024-12-08T14:01:44Z

app/src/main/java/org/sopt/and/presentation/signupScreen/SignUpViewModel.kt

+    private val postSignUpUseCase: PostSignUpUseCase
+) : ViewModel() {
+
+    private val userService by lazy { ServicePool.userService }


ServicePool을 통해 UserService를 가져오는 대신, 생성자를 통한 의존성 주입을 사용하는 것이 좋을 것 같습니다

private val userService by lazy { ServicePool.userService }
↓
@Inject constructor(
private val userService: UserService,
private val postSignUpUseCase: PostSignUpUseCase
)

angryPodo · 2024-12-08T14:02:47Z

app/src/main/java/org/sopt/and/presentation/signupScreen/SignUpViewModel.kt

+        return _isUserNameValid.value && _isPasswordValid.value
+    }
+
+    suspend fun createNewUser() {


createNewUser() 함수가 너무 많은 일을 하고 있는것 같아요. 에러 처리 로직을 별도의 private 함수로 분리하면 코드 가독성이 더 좋아질 것 같습니다. 지금보다 훨씬 깔끔해질 거예요!

angryPodo

지원아 고생 많았다ㅠ
코틀린도 안 익숙한데 이 정도로 적용한 거 보면 진짜 대견하다! 아키텍처 패턴이랑 로직 분리는 나도 아직도 감이 잘 안 오는 부분이라... 앱잼 하면서 실전으로 부딪혀보면 왜 써야 하는지 더 와닿을 거고, 그 다음부터 자연스럽게 발전시켜나갈 수 있을 거야.마지막까지 힘내보자!

jihyunniiii

전체적인 의존성 방향이랑 네비게이션 관련한 내용들 세미나 자료 다시 읽어보시면서 리팩토링 해보면 좋을 것 같아요! 고생하셨습니다.

jihyunniiii · 2024-12-16T22:13:13Z

app/src/main/java/org/sopt/and/domain/usecase/GetUserHobbyUseCase.kt

+import org.sopt.and.data.dataremote.model.response.ResponseGetUserHobbyDto
+import org.sopt.and.domain.repository.UserRepository


domain 레이어인데 data에 대한 의존성을 가지고 있어요.
세미나 때 배운 의존성을 바탕으로 어떻게 수정하면 좋을지 고민해봅시다.

jihyunniiii · 2024-12-16T22:13:42Z

app/src/main/java/org/sopt/and/domain/usecase/GetUserHobbyUseCase.kt

+
+import org.sopt.and.data.dataremote.model.response.ResponseGetUserHobbyDto
+import org.sopt.and.domain.repository.UserRepository
+import retrofit2.Response


마찬가지로 domain 레이어는 순수한 java, kotlin 코드로 이루어져야 합니다.

jihyunniiii · 2024-12-16T22:15:41Z

app/src/main/java/org/sopt/and/presentation/loginScreen/LoginScreen.kt


-    var emailState = remember { mutableStateOf(inputEmail) }
-    var passwordState = remember { mutableStateOf(inputPassword) }
+    var isUserNameValid = loginViewModel.isUserNameValid.collectAsState().value


collectAsState와 collectAsStateWithLifecycle의 차이는 무엇일까요?

collectAsStateWithLifecycle은 백그라운드 상태처럼 화면이 활성화되지 않았을 때는 굳이 State를 계속 관찰하고 있지 않아서 조금 더 효율적으로 상태를 관리할 수 있는 것으로 알고 있습니다!

매번 collectAsStateWithLifecycle을 사용하는 것이 무조건 좋지만은 않을 것 같아서, 구독하고 있는 StateFlow가 얼마나 자주 변경되는 값인지? 등을 고려해서 사용하는 법을 익히는 게 좋을 것 같습니다

jihyunniiii · 2024-12-16T22:16:42Z

app/src/main/java/org/sopt/and/presentation/mypageScreen/MypageViewModel.kt

+import org.sopt.and.data.datalocal.datasource.UserInfoLocalDataSource
+import org.sopt.and.data.dataremote.network.ServicePool


여기도 의존성 다시 한 번 고민해봅시다.

리뷰 반영해서 의존성에 대해 고민해보고 모든 Viewmodel에서 data 레이어 관련 의존성 제거했습니다! (UseCase -> UserRepositoryImpl으로 분리)

jihyunniiii · 2024-12-16T22:17:50Z

app/src/main/java/org/sopt/and/ui/components/BottomBar/NavIcon.kt

+                    navController.navigate(Route.SearchScreen)
+                }
+                "profile" -> {
+                    navController.navigate(Route.MypageScreen(userName = "")) /*username을 여기다 어떻게 넣어주지?*/


Route sealed class를 어떻게 활용하면 값을 넣어줄 수 있을지 생각해보세요!

세미나 자료 참고해보셔요!!

tunaunnie added 14 commits November 13, 2024 02:12

apply MVVM architecture

cda17c8

api connecting

113b9fa

[feat] 유저 등록 API 연동

71d677f

feature #week4 : 회원가입 통신 성공

ee9d131

fix #week4 : signup 후 login 이동 시 navigate 로직 수정

841d04e

fix #week4 : 네이게이션 폴더링 구조 수정

72b791b

feat #week4 : login Get API 구현 완료

5cbd2ee

feat #week4 : 로그인 성공 후 네비게이션

18cd80e

feat #week4 : 취미 조회 API 연결

26c22c1

fix #week4 : NavHost 전체 innerPadding 제거

0512b01

fix #week4 : BaseUrl 숨기기

f6f4b09

initial commit

32230ce

fix #week6 : 폴더 구조 변경

20ecf8f

fix #week6 : DTO request 인자 이름 변경

c3ff78e

tunaunnie requested review from angryPodo, chattymin, jihyunniiii and t1nm1ksun December 6, 2024 09:18

chattymin reviewed Dec 6, 2024

View reviewed changes

t1nm1ksun reviewed Dec 8, 2024

View reviewed changes

fix #week6 : baseurl 숨기기

51c3193

angryPodo reviewed Dec 8, 2024

View reviewed changes

angryPodo approved these changes Dec 8, 2024

View reviewed changes

jihyunniiii reviewed Dec 16, 2024

View reviewed changes

feature #week7 : home 및 login 화면 contract 구현

4f4f579

		private val _loginResult = MutableStateFlow<Boolean?>(null)
		val loginResult = _loginResult.asStateFlow()

-    private val _loginResult = MutableStateFlow<Boolean?>(null)
-    val loginResult = _loginResult.asStateFlow()
+    sealed class LoginUiState {
+    object Initial : LoginUiState()
+    object Success : LoginUiState()
+    data class Error(val message: String) : LoginUiState()
+}
+private val _loginState = MutableStateFlow<LoginUiState>(LoginUiState.Initial)
+val loginState = _loginState.asStateFlow()

		import org.sopt.and.data.dataremote.model.response.ResponseGetUserHobbyDto
		import org.sopt.and.domain.repository.UserRepository

		import org.sopt.and.data.datalocal.datasource.UserInfoLocalDataSource
		import org.sopt.and.data.dataremote.network.ServicePool

Conversation

tunaunnie commented Dec 6, 2024

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

Uh oh!

tunaunnie commented Dec 6, 2024

Uh oh!

chattymin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t1nm1ksun left a comment

Choose a reason for hiding this comment

Uh oh!

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!

angryPodo left a comment

Choose a reason for hiding this comment

Uh oh!

jihyunniiii 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!

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!

5 participants