[Feat] 6주차 필수과제, 심화과제, 도전과제 #11

SYAAINN · 2024-12-04T04:49:21Z

Related issue 🛠

closed [Feat] 6주차 필수과제 , 심화과제 , 도전과제 #10

Work Description ✏️

클린 아키텍쳐 적용
Coroutine을 이용한 서버통신 적용
Hilt를 이용한 DI 구현
나머지 2개 api (검색, 회원정보 변경) 기능구현

Screenshot 📸

검색하기	회원정보 변경
10-search-api.mp4	10-mypage-infochange-api.mp4

Uncompleted Tasks 😅

N/A

To Reviewers 📢

개인적으로 클린 아키텍쳐를 적용하면서 Domain Layer에 선언할 model, type들과 그것들을 Data Layer와 연결해주는 Mapper들 이렇게 2가지를 신경을 많이 쓰는 것 같습니다. 과제 프로젝트이다보니 비즈니스 로직적인 부분이 약해서 그런지 몰라도 이번에 Domain Model을 짜면서 고민을 많이 했습니다. User에 대한 data class로 no, username, password, hobby 등을 하나로 묶어주고 싶었지만 그닥 효율성을 느끼지 못했고 취미 조회 등 Dto를 Domain.User로 매핑하는 과정에서 nullable이나 그런 부차적인 문제들이 생기는 거 같아 포기했습니다. 때문에 User 클래스에 hobby 프로퍼티가 있지만 hobby만을 담는 Hobby라는 클래스도 만들게 되었죠.. 이 부분에서 뭔가 찝찝함이 느껴지는데 다들 어떻게 생각하시나요?!
바텀 네비게이션 탭을 누를때 Scaffold의 content만 바꿔끼우는 방식? 또는 네비게이션으로 이동하는 방식 중 어떤 것이 보편적인지?
전자의 경우, 프로젝트 규모에 따라 바텀 네비게이션을 갖는 뷰의 ViewModel이 너무 무거워질 것 같고(바텀 네비 아이템 스크린들의 뷰모델을 다 포함해야해서?) 후자의 경우, 네비게이션으로 화면 자체를 이동하면 바텀네비게이션 바가 남아있을 지가 궁금합니다 ..!

Marchbreeze

안녕하세요 조장님 :)
저는 XML로만 플젝 7개를 한 외길인생이라 ... 컴포즈 코드에 대해서 잘못 조언을 드릴까봐 팟장님과 얘기하고 다른 멘토님께 양도하기로 얘기를 마쳤었는데, 오랜만에 들어와보니 이전 과제에 코멘트가 없으시네요 ,,,
해명해주세요 @jihyunniiii

이번 주차는 그래도 아키텍쳐적인 부분이 많아 조금 남기고 갑니다 ㅎㅎ

1번 고민에 대해서

단일 책임 원칙 (SRP)에 의하면, Hobby 클래스가 단순히 추가적인 스트링 역할이 아닌 별도의 API를 가지고 있는 것으로 보아, 독립적인 역할을 하는 것으로 보입니다.
이처럼 역할을 가지고 있는 객체의 경우, 추후 확장성을 고려해 따로 클래스를 만드는게 저는 옳은 방향인 것 같습니다.
아니면 하나로 합치고 mapper에서 nullable처리로 진행할 수도 있을 것 같습니다. 다만 이후 확장성을 가지게 되는 경우 mapper가 상당히 무거워질 것 같은 느낌 ,,,

추가로 지금 User Mapper 너무 이쁘네요! 화이팅입니다 ㅎㅎ
참고로 코드 수정 제안은 단순히 제 습관일 뿐이라 적용 안하셔도 괜찮습니다 ~~~

Marchbreeze · 2024-12-04T07:16:06Z

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

+@Module
+@InstallIn(SingletonComponent::class)
+abstract class DataSourceModule {


abstract class와 interface에 대해서 한번 알아보면 도움이 될 것 같습니다 !

Marchbreeze · 2024-12-04T07:18:03Z

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

+fun User.toUserInfoUpdateRequestDto(): UserInfoUpdateRequestDto = UserInfoUpdateRequestDto(
+    hobby = this.hobby,
+    password = this.password
+)


더 코틀린스러운 코드로 만들자면,

멤버 접근 시 기본적으로 this를 생략할 수 있습니다

명시적인 리턴 타입이 없어도 충분히 코드가 명확한 함수입니다 (동일한 네이밍의 Dto가 3번 반복됨)

Suggested change

fun User.toUserInfoUpdateRequestDto(): UserInfoUpdateRequestDto = UserInfoUpdateRequestDto(

hobby = this.hobby,

password = this.password

)

fun User.toUserInfoUpdateRequestDto() = UserInfoUpdateRequestDto(

hobby = hobby,

password = password

)

코드 취향일 뿐이라 그대로 하셔도 됩니다 ~~

Marchbreeze · 2024-12-04T07:20:43Z

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

+fun HobbyResponseDto.toDomain(): Hobby =  Hobby(
+    hobby = this.hobby
+)


저는 이러한 단일 response mapper의 경우, 따로 파일로 관리하지 않고 Dto 아래에

data class HobbyResponseDto( ) { fun toModel() = Hobby( ) }

처럼 활용하는 편입니당

또한 코드 정리가 안되어있는 것 같습니다 (= 뒤에 공백 2개) 습관화하거나 자동화해둡시다 ㅎㅎ

오 ..! data class에 함수를 붙이는 것 참고하겠습니다! 근데 이러니 간단한 mapper는 dto에 붙이고 프로퍼티가 4,5개쯤 되는 dto는 mapper로 분리하자니 일관성이 떨어지는 것 같기도 해서 고민이 되네요 ㅎㅎㅎ

저는 사실 mapper를 안쓰고 다 따로따로 두고있기는 한데 ... ㅎㅎ 재사용성은 위 mapper가 좋아보여서요!

https://github.com/Genti2024/Genti-Android/blob/develop/data/src/main/java/kr/genti/data/dto/response/ImageDto.kt
https://github.com/Genti2024/Genti-Android/blob/develop/data/src/main/java/kr/genti/data/dto/request/CreateRequestDto.kt

저는 이런식으로 관리합니당

저는 아래처럼 주로 사용합니다
dto 를 선언하는 부분에서 아래 처럼 사용합니다.

@Serializable data class ResponseSignInDto( @SerialName("accessToken") val accessToken: String, @SerialName("refreshToken") val refreshToken: String ) fun ResponseSignInDto.toModel() = AuthEntity( accessToken, refreshToken, )

Marchbreeze · 2024-12-04T07:22:22Z

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

+    override suspend fun registerUser(user: User): Result<UserNo> {
+        return runCatching {
+            authRemoteDataSource.registerUser(userRegistrationRequestDto = user.toUserRegistrationRequestDto()).handleApiResponse().getOrThrow().toDomain()
+        }
+    }


더 코틀린스러우려면 return 대신 =를 쓰는게 더 깔끔해보입니다 !

함수형 프로그래밍적인 특성이 잘 드러나는 것 같습니다! return 대신 = 사용을 지향하도록 하겠습니다😀

Marchbreeze · 2024-12-04T07:26:41Z

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

+interface UserRepository {
+    suspend fun getMyHobby(token: String): Result<Hobby>
+    suspend fun getOthersHobby(token: String, userNo: Int): Result<Hobby>
+    suspend fun updateUserInfo(token: String, password: String?, hobby: String?): Result<Unit>
+}


token을 직접 request로 넣어주고 있는건, 아직 토큰인터셉터를 안붙여서일까요?

아직 인터셉터에 대한 지식이 많이 부족해서 이렇게 진행하고 있습니다..! 로그인 시 받는 토큰을 저장해놓고 자동으로 헤더에 붙여준다면 참 편할텐데요 ㅠㅠ

https://github.com/Genti2024/Genti-Android/blob/develop/app/src/main/java/kr/genti/android/di/AuthInterceptor.kt

제 인터셉터 한번 참고해보세요 ~~

안녕하세요. 지나가다가 들렀습니다.

코드를 쭉 읽어보니까 AuthRepository의 login함수에서 로그인 후 Token 객체를 반환하게 해놓으셨는데,
여기서 따로 토큰을 저장하는 로직이 누락된 것 같습니다.
AuthRepositoryImpl의 login 메서드에서 반환타입을 Result<Unit> 으로 하고 로그인 성공 시 tokenRepository의 setToken을 활용하는 방식이 적합해 보입니다.
이러면 호출부에서 로그인 성공여부만 확인하면 되서 책임분리가 가능해집니다.

현재 코드에서는 Authorization Header를 API 호출 시 자동으로 추가하는 기능이 없는 것으로 보입니다.
이걸 하려면 Interceptor Class를 생성해서, 모든 API 요청에 Authorization Header를 자동으로 추가하도록 설정하는게 좋습니다.
참고로, Interceptor는 OkHttp에서 제공하는 기능으로, 네트워크 요청 전/후에 공통 작업을 처리할 수 있는 도구? 입니다.

Interceptor를 사용하려면 Retrofit 설정에서 OkHttpClient의 Builder에 추가해야 합니다.
현재 구조에서는 NetWorkModule에서
.addInterceptor(authInterceptor)
이걸 추가하면 됩니다.

제가 설명을 좀 못한 거 같은데.... 이렇게 한 번 적용해보시고 이해가 안가는 부분이 있으면 연락주세요ㅎ

jihyunniiii · 2024-12-04T07:46:14Z

@Marchbreeze 아니 나 억울해 ㅠㅠ 개인톡으로 말했어 ㅠㅠ

SYAAINN · 2024-12-04T08:18:59Z

@Marchbreeze 계속 리뷰 리퀘스트 날려서 죄송합니다 .. ㅎㅎㅎㅋ 그런데도 이렇게 들어와주셔서 리뷰 남겨주셔서 감사합니다!! 아키텍쳐적인 부분에서 단일 책임 원칙을 고려하는 부분이나, api와도 연결지어서 생각하는 부분은 미처 생각못하고 있었는데 너무 좋은 의견 주신거 같습니다!! 남겨주신 코드적인 부분도 너무 좋은거 같습니다 최대한 반영하는 방향으로 가보겠습니다😀

Marchbreeze · 2024-12-04T10:27:06Z

@Marchbreeze 계속 리뷰 리퀘스트 날려서 죄송합니다 .. ㅎㅎㅎㅋ 그런데도 이렇게 들어와주셔서 리뷰 남겨주셔서 감사합니다!! 아키텍쳐적인 부분에서 단일 책임 원칙을 고려하는 부분이나, api와도 연결지어서 생각하는 부분은 미처 생각못하고 있었는데 너무 좋은 의견 주신거 같습니다!! 남겨주신 코드적인 부분도 너무 좋은거 같습니다 최대한 반영하는 방향으로 가보겠습니다😀

저는 다른 누군가가 봐주는 줄 알았는데 아니어서요..ㅎㅎ 이제 UI 보다는 로직 부분 많이 과제에 있으니 자주 들리겠습니다 많이 물어보세요!! 화이팅

SYAAINN · 2024-12-04T13:50:13Z

@Marchbreeze 계속 리뷰 리퀘스트 날려서 죄송합니다 .. ㅎㅎㅎㅋ 그런데도 이렇게 들어와주셔서 리뷰 남겨주셔서 감사합니다!! 아키텍쳐적인 부분에서 단일 책임 원칙을 고려하는 부분이나, api와도 연결지어서 생각하는 부분은 미처 생각못하고 있었는데 너무 좋은 의견 주신거 같습니다!! 남겨주신 코드적인 부분도 너무 좋은거 같습니다 최대한 반영하는 방향으로 가보겠습니다😀

저는 다른 누군가가 봐주는 줄 알았는데 아니어서요..ㅎㅎ 이제 UI 보다는 로직 부분 많이 과제에 있으니 자주 들리겠습니다 많이 물어보세요!! 화이팅

사실 전 기수 함께했던 분 한분께 요청해놓은 상태라 항상 코드리뷰 해주고 계시긴합니다!! 근데 제가 저번 과제 코드에 신경을 많이 못써서 이번주 코드에 많이많이 달아달라고 요청해놓은 상태입니다😁 앞으로도 놀러와주세요! ㅎㅎ

jangsjw

바쁘신 와중에 심화, 도전까지 다 하다니...리스펙입니다..! 수고하셨습니다!

jangsjw · 2024-12-07T05:22:54Z

app/src/main/java/org/sopt/and/presentation/ui/auth/screen/SignUpScreen.kt

+                    message = "회원가입에 성공했습니다. 유저번호는 ${signUpState.result.no}입니다."
+                )


기존 유저번호 호출 토스트는 현재처럼 Screen이 아닌 어디에서 명령해주고 있었는지 궁금합니다!

jangsjw · 2024-12-07T05:25:42Z

app/src/main/java/org/sopt/and/presentation/ui/main/screen/MainScreen.kt

-    navigateToHome: () -> Unit,
-    navigateToSearch: () -> Unit,
-    navigateToMy: () -> Unit,
+    navigateToSetting: () -> Unit


기존 함수들에서 Setting으로 묶은 이유가 단순 코드의 간결화 때문인지 아니면 기능적 효용성이 있어서 인지 궁금합니다!

cacaocoffee

2번 질문에 대한 답변은 여기로 남깁니다.
말씀주신대로 Scaffold 를 사용할 때 contents 를 변경하는 경우 화면이 많아지고 복잡해질수록 viewmodel 이 너무 비대해지는 문제가 발생해요 간단한 기능에서는 이러한 방식도 사용해도 좋다고 생각이 드네요

navigation을 통해 특정 주소에 대해서 이동을 하는 방식으로 주로 진행을 하게되는 거 같아요 scaffold에서 bottombar 를 이용해서 visable 처리를 하면 이동시 남아있게 할 수 있어요

네비게이션에서 저는 다음과 같은 함수를 두었어요. 각 경로에 따라서 visable 처리를 하기 위함인데요

@Composable
    fun shouldShowBottomBar(): Boolean {
        val currentRoute = currentDestination?.route ?: return false
        return currentRoute in MainNavTab || currentRoute in InMainNavTab || currentRoute.contains("detail") || currentRoute.contains(
            ProfileRoute.route,
        )
    }

위처럼 만든 함수를 아래처럼 사용했어요
bottombar 에 관한 코드를 추가하구 커스텀 뷰를 따로 만들어서 활용했답니다.

저... 코멘트 열심히 달았었는데..ㅠ ㅜ

cacaocoffee · 2024-12-07T17:39:23Z

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

+fun HobbyResponseDto.toDomain(): Hobby =  Hobby(
+    hobby = this.hobby
+)


저는 아래처럼 주로 사용합니다
dto 를 선언하는 부분에서 아래 처럼 사용합니다.

@Serializable data class ResponseSignInDto( @SerialName("accessToken") val accessToken: String, @SerialName("refreshToken") val refreshToken: String ) fun ResponseSignInDto.toModel() = AuthEntity( accessToken, refreshToken, )

cacaocoffee · 2024-12-07T17:45:54Z

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

+data class User(
+    val username: String,
+    val password: String,
+    val hobby: String,


이 부분은 확장성을 고려한다면 hobby 클래스로 받는게 좋아보여요
예시로는 다음과 같이요

Suggested change

val hobby: String,

val hobby: Hobby,

1번 질문에 대한 제 생각으로는 hobby라는 클래스를 따로 두는 것은 좋은거 같아요.

추후 취향을 반영할 때 취향에 대한 이름 뿐만 아니라 여러가지 요소가 들어가는 방향으로 진행된다면 hobby data class 를 변경하고 유저에 따라 취향에 관한 것을 한번에 관리할 수 있을 거 같네요

hwidung

우와어... 이번 주도 고생 많으셨습니다..! 어려운게 많은데 질문 많이 많이 할게요. ..ㅎㅎ

hwidung · 2024-12-08T09:20:31Z

app/src/main/java/org/sopt/and/presentation/ui/auth/screen/AuthState.kt

저는 이전에 이런 로딩, 성공, 실패 등을 뷰모델에서 관리했었던 적이 있었는데 ...ㅎ 이렇게 AuthState에서 따로 관리를 하는 것이 좋아보이네요 !!

hwidung · 2024-12-08T09:30:42Z

app/src/main/java/org/sopt/and/presentation/ui/main/screen/MainViewModel.kt

-                    }
+            _userHobbyState.value = UserHobbyState.Loading
+            val result = userRepository.getMyHobby(token = token)
+            _userHobbyState.value = result.fold(


설명해준 체이닝 메서드 부분이구나 !! 설명해준 덕분에 이해가 쉬워졌어 증말로 고마워요 ~~

jihyunniiii

고생하셨습니다 ~

jihyunniiii · 2024-12-17T07:06:51Z

app/src/main/java/org/sopt/and/presentation/ui/auth/screen/AuthViewModel.kt

    private val _signInState = MutableStateFlow<SignInState>(SignInState.Idle)
    val signInState: StateFlow<SignInState> = _signInState

-    private val _signInUserName = MutableLiveData("")
-    val signInUserName: LiveData<String> = _signInUserName
-
-    private val _signInPassword = MutableLiveData("")
-    val signInPassword: LiveData<String> = _signInPassword
+    private val _token = MutableStateFlow("")
+    val token: StateFlow<String> = _token


StateFlow와 Shared Flow의 차이점을 이해해보시고 적절한 것을 적용해보시면 좋을 것 같네요 ~

jihyunniiii · 2024-12-17T07:08:23Z

app/src/main/java/org/sopt/and/presentation/ui/auth/screen/SignInScreen.kt

 @Composable
 fun SignInScreen(
+    signInState: SignInState,
+    resetSignInState: () -> Unit,


SignInRoute와 SignInScreen을 분리하신 이유가 무엇일까요?
각각은 어떤 책임을 가지면 좋을까요?

jihyunniiii · 2024-12-17T07:11:12Z

app/src/main/java/org/sopt/and/presentation/ui/main/screen/MyPageScreen.kt

+    mainViewModel: MainViewModel = hiltViewModel(),
+    navigateToSetting: () -> Unit
+) {
+    val userHobbyState by mainViewModel.userHobbyState.collectAsState()


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

SYAAINN added 16 commits December 2, 2024 11:41

[feat/#10] api 5개를 위한 service와 model을 정의합니다.

935d201

[feat/#10] datasource를 구현합니다.

f48e44e

[feat/#10] repository를 구현합니다.

daf06f5

[feat/#10] 의존성 주입을 위한 module을 생성합니다.

22edaeb

[feat/#10] SignIn, SignUp 스크린의 서버통신을 Coroutine 방식으로 변경합니다.

663c08d

[feat/#10] 서버통신 방식을 변경하고, 검색하기와 회원정보변경 페이지를 구현합니다.

d1e6549

[feat/#10] SettingScreen 라우트를 추가합니다.

1b87635

[feat/#10] 공통 컴포넌트로 변경합니다.

48f7acb

[feat/#10] 취미 검색 통신 상태입니다.

6c61aae

[chore/#10] 선언 위치를 수정합니다.

797bbda

[chore/#10] Loading State로 설정하는 위치를 변경합니다.

66a5a90

[chore/#10] 검색 화면 UI를 개선합니다.

4720867

[chore/#10] 회원정보 변경 화면 UI를 개선합니다.

dc24357

[chore/#10] 변수명을 email -> username으로 변경합니다.

0d19161

[feat/#10] response body가 없는 경우 Response 객체를 이용하여 서버통신 상태를 검사합니다.

189f674

[chore/#10] UI를 개선합니다.

e298d38

SYAAINN requested a review from a team December 4, 2024 04:49

SYAAINN self-assigned this Dec 4, 2024

SYAAINN requested review from cacaocoffee, Marchbreeze, jihyunniiii and hwidung and removed request for a team December 4, 2024 04:49

Marchbreeze reviewed Dec 4, 2024

View reviewed changes

jangsjw approved these changes Dec 7, 2024

View reviewed changes

cacaocoffee approved these changes Dec 7, 2024

View reviewed changes

hwidung 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

[Feat] 6주차 필수과제, 심화과제, 도전과제 #11

[Feat] 6주차 필수과제, 심화과제, 도전과제 #11

SYAAINN commented Dec 4, 2024

Marchbreeze left a comment

Marchbreeze Dec 4, 2024

Marchbreeze Dec 4, 2024

Marchbreeze Dec 4, 2024

SYAAINN Dec 4, 2024

Marchbreeze Dec 4, 2024

cacaocoffee Dec 7, 2024

Marchbreeze Dec 4, 2024

SYAAINN Dec 4, 2024

Marchbreeze Dec 4, 2024

SYAAINN Dec 4, 2024

Marchbreeze Dec 4, 2024

MoonsuKang Dec 4, 2024

jihyunniiii commented Dec 4, 2024

SYAAINN commented Dec 4, 2024

Marchbreeze commented Dec 4, 2024

SYAAINN commented Dec 4, 2024

jangsjw left a comment

jangsjw Dec 7, 2024

jangsjw Dec 7, 2024

cacaocoffee left a comment

cacaocoffee Dec 7, 2024

cacaocoffee Dec 7, 2024

hwidung left a comment

hwidung Dec 8, 2024

hwidung Dec 8, 2024

jihyunniiii left a comment

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

jihyunniiii Dec 17, 2024

		message = "회원가입에 성공했습니다. 유저번호는 ${signUpState.result.no}입니다."
		)

[Feat] 6주차 필수과제, 심화과제, 도전과제 #11

Are you sure you want to change the base?

[Feat] 6주차 필수과제, 심화과제, 도전과제 #11

Conversation

SYAAINN commented Dec 4, 2024

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

Marchbreeze 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihyunniiii commented Dec 4, 2024

SYAAINN commented Dec 4, 2024

Marchbreeze commented Dec 4, 2024

SYAAINN commented Dec 4, 2024

jangsjw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cacaocoffee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwidung left a 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