Week6 #10

sayyyho · 2024-12-08T11:01:51Z

Related issue 🛠

closed [Week6] 6주차 과제 구현 #9

Work Description ✏️

clean architecture 적용

📦org.sopt.and
├─📂data
│  ├─📂datasource
│  ├─📂mapper
│  ├─📂model
│  │  ├─📂request
│  │  └─📂response
│  ├─📂repositoryimpl
│  ├─📂service
├─📂domain
│  ├─📂model
│  ├─📂repository
│  ├─📂usecase
├─📂presentation
│  ├─📂components
│  ├─📂navigation
│  ├─📂home
│  ├─📂myinfo
│  ├─📂search
│  ├─📂signin
│  ├─📂signup
│  ├─📂util
│  ├─📂viewmodelfactory
└─📂ui.theme

To Reviewers 📢

제가 너무 늦게 제출해서 리뷰에 차질 생긴 점도 정말 죄송합니다.
또, 항상 꼼꼼하게 리뷰해주셔서 정말 감사드립니다.

늘 스스로 더 고민하지 못하고 다른 분들께 여쭤보고 또, 코드 보고 많이 도움받아서 겨우겨우 구현을 하네요ㅠㅠ
여전히 여러 개념들을 이해하지 못한 거 같아서 부족한 점도 다시금 느끼네요 😂

다들 바쁘실텐데 다른 핑계를 대기엔, 결국 제 공부 부족이 원인인 걸 잘 알고 있습니다ㅠㅠ
도움받았던 코드들과 코드 리뷰 피드백을 토대로 이번주는 열심히 공부하겠습니다.

다음엔 조금 더 스스로 해결하고 제대로 된 코드로 보여드릴 수 있도록 노력하겠습니다.
늘 귀한 시간 내주셔서, 다시 한 번 감사합니다 🥹

hyoeunjoo

뭔가 대대적인 큰 변화가 있는데요??! 이번주 과제도 수고 많으셨습니다

hyoeunjoo · 2024-12-08T14:04:59Z

app/src/main/java/org/sopt/and/data/mapper/Mapper.kt

+
+object Mapper {
+    fun toMyHobbyEntity(getHobbyResponseResultDto: MyHobbyResponseResultDto) =
+        MyHobbyEntity(myHobby = getHobbyResponseResultDto.myHobby)
+
+    fun toSignUpResponseEntity(signUpResponseDto: Response<SignUpResponseDto>) =
+        signUpResponseDto.body()?.result?.let {
+            SignUpResponseEntity(
+                no = it.no,
+                status = signUpResponseDto.code(),
+                code = signUpResponseDto.body()!!.code
+            )
+        }
+
+    fun toSignInResponseEntity(signInResponseDto: Response<SignInResponseDto>) =
+        signInResponseDto.body()?.result?.let {
+            SignInResponseEntity(
+                token = it.token,
+                status = signInResponseDto.code(),
+                code = signInResponseDto.body()!!.code
+            )
+        }
+


매퍼 object에 모든 매퍼를 넣어놓으신 이유가 있을까요?? 분리해주셔도 좋을 것 같습니다!

저도 궁금합니다 ~

hyoeunjoo · 2024-12-08T14:19:27Z

app/src/main/java/org/sopt/and/data/model/response/MyHobbyResponseDto.kt

+@Serializable
+data class MyHobbyResponseDto(
+    val result: MyHobbyResponseResultDto? = null,
+    val code: String? = null
+)


Dto는 명세서에서 Response Body나 Request Body값을 보고 만들면 됩니다!

@Serializable data class MyHobbyResponseResultDto( @SerialName("hobby") val myHobby: String )

아래 적어주신 이 부분만을 Dto로 남겨주시고 지금 result, code는 따로 BaseResponse형태를 만들어주시는게 더 나아보입니다!

hyoeunjoo · 2024-12-08T14:24:33Z

app/src/main/java/org/sopt/and/data/repositoryimpl/MyHobbyRepositoryImpl.kt

+    override suspend fun getMyHobby(): Result<MyHobbyEntity> =
+        runCatching {
+            getMyHobbyDataSource.getMyHobby().result?.let { Mapper.toMyHobbyEntity(it) }!!
+        }


!!은 이왕이면 사용하지 않는 것이 좋습니다 ㅎㅎ NPE를 불러올 수 있기 때문이죠
데이터가 없을 경우 명시적으로 처리하도록 수정하는 것이 좋아보입니다

hyoeunjoo · 2024-12-08T14:26:44Z

app/src/main/java/org/sopt/and/data/service/UserService.kt

+interface UserService {
+    suspend fun signUp(@Body request: SignUpRequestDto): Response<SignUpResponseDto>


여기는 왜 Post 어노테이션이 없나요??

hyoeunjoo · 2024-12-08T14:29:04Z

app/src/main/java/org/sopt/and/domain/model/SignInResponseEntity.kt

+
+data class SignInResponseEntity(
+    val token: String? = null,
+    val status: Int? = null,
+    val code: String? = null
+)


domain에 있는 Entity는 무엇일까요? 어떤 역할을 하는걸까요??

hyoeunjoo · 2024-12-08T14:37:05Z

app/src/main/java/org/sopt/and/presentation/signin/SignInViewModel.kt

+                if (signInResponseEntity.status == 200) {
+                    _signInResult.value = SignInResult.Success
+                    signInResponseEntity.token?.let { token ->
+                        tokenManager.saveToken(token)
+                    }
+                } else if (signInResponseEntity.code == SignInFailureCase.FAILURE_LENGTH.errorCode
+                    && signInResponseEntity.status == SignInFailureCase.FAILURE_LENGTH.statusCode
+                ) {
+                    _signInResult.value = SignInResult.FailurePasswordLength
+                } else if (signInResponseEntity.code == SignInFailureCase.FAILURE_WRONG_PASSWORD.errorCode
+                    && signInResponseEntity.status == SignInFailureCase.FAILURE_WRONG_PASSWORD.statusCode
+                ) {
+                    _signInResult.value = SignInResult.FailureWrongPassword
+                }
+            }


fun signUp(request: RequestSignUpDto) { viewModelScope.launch { runCatching { authService.signUp(request) }.onSuccess { // 성공 시 로직 }.onFailure { // 실패 시 로직 } } }

else if문을 쓰기보단 이렇게 쓰는게 더 가독성 좋을 것 같다는 생각이 들어요
세미나 코드 발췌

hyoeunjoo · 2024-12-08T14:39:44Z

app/src/main/java/org/sopt/and/presentation/util/Utils.kt

+
+    fun Context.showToast(
+        @StringRes message: Int
+    ) = Toast.makeText(
+        this,
+        this.getString(message),
+        Toast.LENGTH_SHORT
+    ).show()
+
+    fun Context.showSnackbar(
+        scope: CoroutineScope,
+        snackbarHostState: SnackbarHostState,
+        @StringRes message: Int
+    ) = scope.launch {
+        snackbarHostState.showSnackbar(message = getString(message))
+    }
+}


이런건 따로 확장함수 폴더로 빼주시면 나중에 비슷한 확장함수 추가할때도 더 빠르고 쉽게 찾을 수 있을 것 같아용

hyoeunjoo · 2024-12-08T14:41:51Z

app/src/main/java/org/sopt/and/presentation/viewmodelfactory/SignUpViewModelFactory.kt

+
+class SignUpViewModelFactory : ViewModelProvider.Factory {
+    override fun <T : ViewModel> create(modelClass: Class<T>): T {
+        return when (modelClass) {
+
+            SignUpViewModel::class.java -> {
+                SignUpViewModel(
+                    SignUpUseCase(
+                        signUpRepository = SignUpRepository.create()
+                    )
+                ) as T
+            }
+
+            else -> throw IllegalArgumentException("Unknown ViewModel Class")
+        }
+    }
+}


ViewModelProvider.Factory를 상속받는 Factory들은 각 뷰모델 별로 만들 필요는 없습니다!
하나의 ViewModel Class 안에 if else 혹은 when문을 통해 각 뷰모델과 repository 별로 분기처리 하시면 됩니다!

imtaejugkim

정말 많은 노력의 흔적들이 보입니다...! 데이터, 도메인, 프레젠테이션 각각 생각하시면서 코드를 구성 하신게 보입니다. 클린 아키텍쳐를 적용시키는게 저를 포함해서 아마 많이 어려우셨을 거라 생각해요.. 어려웠던 만큼, 더욱 도움이 됐던 주차였길 바랍니다!

imtaejugkim · 2024-12-08T14:48:17Z

app/src/main/java/org/sopt/and/data/repositoryimpl/MyHobbyRepositoryImpl.kt

+    private val getMyHobbyDataSource: MyHobbyDataSource
+) : MyHobbyRepository {
+    override suspend fun getMyHobby(): Result<MyHobbyEntity> =
+        runCatching {


runCatching 굿입니다!

imtaejugkim · 2024-12-08T14:49:58Z

app/src/main/java/org/sopt/and/data/service/TokenManager.kt

+val Context.dataStore by preferencesDataStore(DATASTORE_NAME)
+
+class TokenManager(private val context: Context) {
+    private val TOKEN_KEY = stringPreferencesKey(TOKEN_NAME)
+
+    fun getToken(): Flow<String?> {
+        return context.dataStore.data.map { preferences ->
+            preferences[TOKEN_KEY]
+        }
+    }
+
+    suspend fun saveToken(token: String) {
+        context.dataStore.edit { preferences ->
+            preferences[TOKEN_KEY] = token
+        }
+    }
+
+    companion object {
+        const val DATASTORE_NAME = "token"
+        const val TOKEN_NAME = "auth_token"
+    }
+}


오오 data store을 사용하셨군요!
shared Preference와 다르게 Data Store가 요즘 더 권장? 추천? 하는 방식이라고 얼핏 들은것 같습니다!
둘의 어떤 장단점의 차이가 있나요??

imtaejugkim · 2024-12-08T14:52:25Z

app/src/main/java/org/sopt/and/presentation/viewmodelfactory/MyInfoViewModelFactory.kt

+class MyInfoViewModelFactory : ViewModelProvider.Factory {
+    override fun <T : ViewModel> create(modelClass: Class<T>): T {
+        return when (modelClass) {
+
+            MyInfoViewModel::class.java -> {
+                MyInfoViewModel(
+                    MyHobbyUseCase(
+                        getMyHobbyRepository = MyHobbyRepository.create()
+                    )
+                ) as T
+            }
+
+            else -> throw IllegalArgumentException("Unknown ViewModel Class")
+        }
+    }
+}


hilt를 안쓸 때의 viewModel 주입 방법인가요?

imtaejugkim · 2024-12-08T14:54:38Z

app/src/main/java/org/sopt/and/domain/model/SignInResponseEntity.kt

저도 이번 피드백 내용이었는데.. domain단에는 response와 request와 같은, 서버에서 사용한 dto 형식의 네이밍은 좋지 않다고 들었어요!

jihyunniiii

고생하셨습니다 ~

jihyunniiii · 2024-12-17T08:44:53Z

app/src/main/java/org/sopt/and/data/mapper/Mapper.kt

+
+object Mapper {
+    fun toMyHobbyEntity(getHobbyResponseResultDto: MyHobbyResponseResultDto) =
+        MyHobbyEntity(myHobby = getHobbyResponseResultDto.myHobby)
+
+    fun toSignUpResponseEntity(signUpResponseDto: Response<SignUpResponseDto>) =
+        signUpResponseDto.body()?.result?.let {
+            SignUpResponseEntity(
+                no = it.no,
+                status = signUpResponseDto.code(),
+                code = signUpResponseDto.body()!!.code
+            )
+        }
+
+    fun toSignInResponseEntity(signInResponseDto: Response<SignInResponseDto>) =
+        signInResponseDto.body()?.result?.let {
+            SignInResponseEntity(
+                token = it.token,
+                status = signInResponseDto.code(),
+                code = signInResponseDto.body()!!.code
+            )
+        }
+


저도 궁금합니다 ~

jihyunniiii · 2024-12-17T08:45:10Z

app/src/main/java/org/sopt/and/data/mapper/Mapper.kt

+            SignUpResponseEntity(
+                no = it.no,
+                status = signUpResponseDto.code(),
+                code = signUpResponseDto.body()!!.code


!!는 NPE가 발생할 수 있으니 사용을 지양하시는 것이 좋습니다.

jihyunniiii · 2024-12-17T08:46:04Z

app/src/main/java/org/sopt/and/data/service/AppContext.kt

data layer에 위치시키신 이유가 있나요?

jihyunniiii · 2024-12-17T08:47:01Z

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

+import org.sopt.and.data.datasource.SignInDataSource
+import org.sopt.and.data.repositoryimpl.SignInRepositoryImpl
+import org.sopt.and.data.service.ServicePool
+import org.sopt.and.domain.model.SignInInformationEntity
+import org.sopt.and.domain.model.SignInResponseEntity


domain은 어떠한 외부 레이어와도 의존성이 있으면 안 됩니다.
수정해보시면 좋을 것 같네요

jihyunniiii · 2024-12-17T08:48:06Z

app/src/main/java/org/sopt/and/presentation/myinfo/MyInfoViewModel.kt

+    private val _uiState = MutableStateFlow(MyInfoUiState())
+    val uiState: StateFlow<MyInfoUiState> = _uiState.asStateFlow()


StateFlow와 SharedFlow의 차이는 무엇일까요?

sayyyho added 11 commits December 8, 2024 17:56

[chore] #9 - 파일 구조 변경

b766625

[chore] #9 - 파일 구조 변경

a346adb

[feature] #9 - repository, usecase 구현

7a7e46b

[chore] #9 - gradle 동기화

9e4b874

[feature] #9 - myInfo 재구현

70cb373

[feature] #9 - SignIn 재구현

466c5c6

[fix] #9 - 공통 컴포넌트 수정

35afaca

[fix] #9 - SignUp / SignIn 구조 수정

bb69bad

[fix] #9 - 빌드 에러 수정

f7f0610

[fix] #9 - 홈 수정

778838d

[fix] #9 - AndroidManifest 수정

a760cb1

sayyyho requested review from imtaejugkim, jihyunniiii and hyoeunjoo December 8, 2024 11:03

sayyyho self-assigned this Dec 8, 2024

hyoeunjoo requested changes Dec 8, 2024

View reviewed changes

imtaejugkim approved these changes Dec 8, 2024

View reviewed changes

jihyunniiii reviewed Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Week6 #10

Week6 #10

sayyyho commented Dec 8, 2024 •

edited

Loading

hyoeunjoo left a comment

hyoeunjoo Dec 8, 2024

jihyunniiii Dec 17, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

hyoeunjoo Dec 8, 2024

imtaejugkim left a comment

imtaejugkim Dec 8, 2024

imtaejugkim Dec 8, 2024

imtaejugkim Dec 8, 2024

imtaejugkim Dec 8, 2024

jihyunniiii left a comment

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

		interface UserService {
		suspend fun signUp(@Body request: SignUpRequestDto): Response<SignUpResponseDto>

		private val _uiState = MutableStateFlow(MyInfoUiState())
		val uiState: StateFlow<MyInfoUiState> = _uiState.asStateFlow()

Week6 #10

Are you sure you want to change the base?

Week6 #10

Conversation

sayyyho commented Dec 8, 2024 • edited Loading

Related issue 🛠

Work Description ✏️

To Reviewers 📢

hyoeunjoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imtaejugkim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihyunniiii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayyyho commented Dec 8, 2024 •

edited

Loading