[Week4] 4주차 필수 과제 #12

hyeeum · 2024-11-14T18:08:31Z

Related issue 🛠

closed [Feature] 4주차 필수 과제 #11

Work Description ✏️

[유저 등록]
- 회원가입 페이지에 진행해주세요.
- 기존에 이메일, 비밀번호를 받는 구조에서 username, password, hobby를 입력하는 구조로 변경해주세요.
- username, password, hobby가 조건을 만족하지 않는 경우 (8자 이상인 경우) 조건을 만족하지 않았다는 것을 화면에 표시해주세요. (방식은 자율입니다.)

[로그인]
- 로그인 페이지에 진행해주세요.
- 기존 이메일, 비밀번호를 받는 구조에서 username, password를 받는 구조로 변경해주세요.
- 로그인 실패, 성공 시에 알맞은 안내 문구를 유저에게 제시해주세요. (방식은 자율입니다.)

[내 취미 조회]
- MY 페이지에 진행해주세요
- 기존에 이메일을 표시했던 부분에 내 취미가 표시되도록 해주세요.

Screenshot 📸

이름 오류	비밀번호 오류	취미 오류	정상 회원가입

로그인 오류	정상 로그인

취미 불러오기

To Reviewers 📢

네이밍 붙이는게 참어려운 것 같아요 , signup signin을 data layer코드에 넣는게 맞는지 뭔가 하면서도 이건 아닌 것 같다는 느낌이 드는데 다른 분들의 코드를 많이 참고하면서 더 좋은 네이밍을 알아보도록 하겠습니다 : - )
그리고 서버통신은 한 번 하면 감이 잡히기는하는데 체화를 하는 과정이 참 오래 걸리는 것 같아요 패키지를 분리하는 과정에서 다시 한 번 아키텍처 정리를 할 수 있었습니다..! 까먹지않도록 계속 봐야할 것 같네요

greedy0110

네트워크 연결 과제 고생 많으셧습니다 혜음님, 간단히 코멘트 남겨두었어요.

greedy0110 · 2024-11-16T09:01:10Z

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

+        return OkHttpClient.Builder()
+            .addInterceptor(loggingInterceptor)
+            .addInterceptor(authInterceptor)
+            .build()


릴리즈에는 필요없는 interceptor이므로 디버그 앱에만 추가해주심이 어떨까요

Suggested change

return OkHttpClient.Builder()

.addInterceptor(loggingInterceptor)

.addInterceptor(authInterceptor)

.build()

val builder = OkHttpClient.Builder()

if (BuildConfig.DEBUG) builder.addInterceptor(loggingInterceptor)

builder.addInterceptor(authInterceptor)

return builder.build()

greedy0110 · 2024-11-16T09:05:31Z

app/src/main/java/org/sopt/and/feature/my/MyViewModel.kt

+        viewModelScope.launch {
+            wavveRepository.getHobby().onSuccess { hobbyEntity ->
+                _state.value = _state.value.copy(
+                    hobby = hobbyEntity.hobby
+                )
+            }.onFailure { }


실패하면 사용자는 아무런 피드백을 받지 못하고 빈 화면을 보게 될 것 같습니다.
앱을 사용하는 사람 입장으로 생각하면 피하고 싶은 상황일 것 같아요.
최소한 토스트로 문제가 생겼음을 보여주거나 뭔가 문제가 있으니 재시도 하라는 화면이 있으면 좋을 것 같습니다.

greedy0110 · 2024-11-16T09:08:34Z

app/src/main/java/org/sopt/and/sharedpreference/User.kt

+import javax.inject.Singleton
+
+@Singleton
+class User @Inject constructor(


모듈이 분리된 이 시점에서 이 객체는 다분히 data 혹은 repository 수준에서 관리될 친구로 보이네요.

data로 관리한다면 UserToken 정보를 디바이스에 저장하는 객체로 변모할 것이고 (현재 프로젝트 구조 상 이름은 UserDataSource, TokenDataSource ?)

repository로 관리한다면 UserToken 정보를 저장하고, 필요한 시점에 초기화 하는 인터페이스를 제공하는 객체로 활용되겠네요. (현재 프로젝트 구조에 맞는 이름은 아마도 UserRepository? AuthRepository? SignRepository)

kamja0510

역시 혜음.. 저같은 사람도 코드를 알아볼 수 있게 짜셨네요(mvi는 살짝 이해못함 ㅎ)
항상 코드리뷰 시간이 아니라 보면서 공부하는 느낌

kamja0510 · 2024-11-17T08:07:01Z

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

+    @Provides
+    fun provideLoggingInterceptor(): HttpLoggingInterceptor {
+        return HttpLoggingInterceptor { message ->
+            Log.d("Retrofit2", "CONNECTION INFO -> $message")


세미나에서 배운것은 이 중괄호 블럭이 없는데 이 블럭이 추가되면서 어떤 추가적인 정보를 볼 수 있나요?

kamja0510 · 2024-11-17T08:08:19Z

app/src/main/java/org/sopt/and/data/dto/request/RequestSignUpDto.kt

+
+@Serializable
+data class RequestSignUpDto(
+    @SerialName("username")


변수명과 json key명이 같은데도 SerialName 어노테이션을 굳이 써야하나요?

kamja0510 · 2024-11-17T08:09:19Z

app/src/main/java/org/sopt/and/data/dto/response/ResponseUserHobbyDto.kt

+    @SerialName("hobby")
+    val hobby: String
+){
+    fun toEntity() = ResponseHobbyEntity(


신기한 문법들

kamja0510 · 2024-11-17T08:13:11Z

app/src/main/java/org/sopt/and/feature/search/SearchNavigation.kt


 fun NavController.navigateSearch(
    navOptions: NavOptions
 ) {
-    navigate(Search, navOptions)
+    navigate(org.sopt.and.feature.search.Search, navOptions)


오잉 왜 갑자기 경로 다 줘야되는걸로 바뀌었나요

kamja0510 · 2024-11-17T08:40:30Z

app/src/main/java/org/sopt/and/feature/signin/SignInState.kt

+    val password: String = "",
+    var isPasswordVisible: Boolean = false,
+) {
+    val isButtonEnabled: Boolean = username.isNotEmpty() && password.isNotEmpty()


이 줄이 왜 중괄호 블럭으로 따로 빠져 있나요? 저 username이랑 password를 쓰려면 따로 빼야하는 건가요?

kamja0510 · 2024-11-17T08:43:44Z

app/src/main/java/org/sopt/and/feature/signup/SignUpViewModel.kt

@@ -72,9 +94,8 @@ class SignUpViewModel @Inject constructor() : ViewModel() {
    }

    companion object {
-        private const val MIN_PASSWORD = 8
-        private const val MAX_PASSWORD = 20
+        private const val MIN_SIGNUP_LENGTH = 1


와 저는 상수화도 안하고 최소값1로 두지도 않았네요 혼나겠다

kamja0510 · 2024-11-17T08:45:31Z

app/src/main/java/org/sopt/and/sharedpreference/User.kt

+@Singleton
+class User @Inject constructor(
+    @ApplicationContext private val context: Context
+) {
    private val sharedPreferences: SharedPreferences =


호옥시 datastore 안쓰시고 sharedPreferences 쓰시는 이유가 있나요!

kamja0510 · 2024-11-17T08:47:02Z

app/src/main/res/values/strings.xml

사소한 부분들도 string들을 다 바꾸셨네요!

kamja0510 · 2024-11-17T08:51:38Z

app/src/main/java/org/sopt/and/feature/my/MyRoute.kt

 ) {
+    val state by viewModel.state.collectAsStateWithLifecycle()
+
+    LaunchedEffect(true) {


이런거 보면 Route를 쓰는게 좋을 거 같다는 생각이 드네요

HAJIEUN02

고생많았서요 혜음이~~ 클린아키텍처 적용을 해서 그런지 저랑 비슷한 부분이 많아서 보기 편했습니다 ㅎㅎ 유스케이스도 한번 사용해보셔요!

HAJIEUN02 · 2024-11-17T09:39:50Z

app/build.gradle.kts

+
+    // Network
+    implementation(platform(libs.okhttp.bom))
+    implementation(libs.okhttp)


okhttp랑 retrofit처럼 같은 카테고리인 친구들은 각각 libs.versions.toml 내부에서 bundles로 묶어서 한번에 디펜던시 추가해줄 수 있서요! 그러면 더 보기 깔끔할 듯합니당

HAJIEUN02 · 2024-11-17T09:40:25Z

app/src/main/AndroidManifest.xml

        tools:targetApi="31">
        <activity
-            android:name=".main.MainActivity"
+            android:name=".feature.main.MainActivity"


오호 ui가 아니라 feature로 패키지를 구성하셨군요
새로운 네이밍 좋네요

HAJIEUN02 · 2024-11-17T09:42:36Z

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

+    @Provides
+    fun provideLoggingInterceptor(): HttpLoggingInterceptor {
+        return HttpLoggingInterceptor { message ->
+            Log.d("Retrofit2", "CONNECTION INFO -> $message")


로그는 PR 올릴 때 삭제해주시거나 Timber로 바꿔주시면 좋을 것 같아요!!

HAJIEUN02 · 2024-11-17T10:19:37Z

app/src/main/java/org/sopt/and/data/dto/request/RequestSignInDto.kt

+    val password: String,
+)
+
+fun RequestSignInEntity.toDto() = RequestSignInDto(


요거는 엔티티를 디티오로 바꾸는 거니까 엔티티가 주체잖아요!! 그래서 저라면 엔티티에 해당함수를 구현할 것 같아요

1971123-seongmin

고생하셨습니다. 항상 잘하셔서 리뷰라기보단 많이 배워가는 것 같아요.

1971123-seongmin · 2024-11-17T14:16:20Z

app/src/main/java/org/sopt/and/data/datasourceimpl/WavveDataSourceImpl.kt

+    override suspend fun getUserHobby(): BaseResponse<ResponseUserHobbyDto> =
+        wavveService.getUserHobby()
+
+}


깔끔 그 자체에요. 코드 너무 좋아요

1971123-seongmin · 2024-11-17T14:16:55Z

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

+    @Binds
+    @Singleton
+    abstract fun bindsDataSource(myDataSourceImpl: WavveDataSourceImpl): WavveDataSource
+}


데이터스토어 모듈을 따로 만드셨네요. 저도 분리해야겠어요.

1971123-seongmin · 2024-11-17T14:19:32Z

app/src/main/java/org/sopt/and/data/dto/response/ResponseSignInDto.kt

+    @SerialName("token")
+    val token: String
+){
+    fun toEntity() = ResponseSignInEntity(


objcect에 Mapper를 모아두는 방식을 안쓰고 데이터 클래스에 바로 정의하신 이유 알려주실 수 있나요??

1971123-seongmin · 2024-11-17T14:21:22Z

app/src/main/java/org/sopt/and/feature/signup/SignUpNavigation.kt


 import androidx.navigation.NavController
 import androidx.navigation.NavGraphBuilder
 import androidx.navigation.compose.composable
 import kotlinx.serialization.Serializable
-import org.sopt.and.main.MainTabRoute
+import org.sopt.and.feature.main.MainTabRoute

 fun NavController.navigateSignUp() {
    navigate(SignUp)


컴포즈도 보통 이렇게 네비게이션 함수를 확장으로 두고 쓰나요??

1971123-seongmin · 2024-11-17T14:21:59Z

app/src/main/java/org/sopt/and/feature/signup/SignUpState.kt

+    val hobby: String = "",
+    var isPasswordVisible: Boolean = false,
+) {
+    val isButtonEnabled: Boolean = username.isNotEmpty() && password.isNotEmpty()


이렇게 하면 버튼 활성화/비활성화를 관리할 때 훨씬 편할까요??

1971123-seongmin · 2024-11-17T14:23:19Z

app/src/main/res/values/strings.xml

    <string name="not_valid_input">유효하지 않은 입력입니다.</string>

    <!--signin-->
    <string name="signin">로그인</string>
-    <string name="signin_placeholder">이메일 주소 또는 아이디</string>
+    <string name="signin_username_placeholder">이름</string>
+    <string name="signin_password_placeholder">비밀번호</string>
    <string name="find_id">아이디 찾기</string>
    <string name="reset_password">비밀번호 재설정</string>
    <string name="join">회원가입</string>


string 이름들이 정말 깔끔하고 직관적이네요..

jihyunniiii

역시 참 깔끔하네요! 고생하셨습니다! 우리 혜음이 합세도 파이팅!

jihyunniiii · 2024-11-19T19:58:18Z

app/src/main/java/org/sopt/and/data/datasourceimpl/WavveDataSourceImpl.kt

+import javax.inject.Inject
+
+class WavveDataSourceImpl @Inject constructor(
+    val wavveService: WavveService


private를 붙여줘도 좋을 듯,,!! 합니다

jihyunniiii · 2024-11-19T19:59:49Z

app/src/main/java/org/sopt/and/data/dto/response/BaseResponse.kt

+
+

요기는 2줄 공백이당 크크

jihyunniiii · 2024-11-19T20:04:37Z

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

 import javax.inject.Inject

 @HiltViewModel
-class SignInViewModel @Inject constructor() : ViewModel() {
+class SignInViewModel @Inject constructor(
+    private val user:User,


정렬이 제대로 안 된 것 같숨다

그리고 여기 user를 매개변수로 넣어주는 이유가 뭔가요? ㅜ 찾고 싶은데 졸려서 그런가 안 보여잉 ㅠㅠㅠ

jihyunniiii · 2024-11-19T20:10:31Z

app/src/main/java/org/sopt/and/sharedpreference/User.kt

+import javax.inject.Singleton
+
+@Singleton
+class User @Inject constructor(


아 위에 있는 user가 이러한 역할을 하는 친구였군요!
승민님 리뷰처럼 조금 더 직관적으로 네이밍하는 게 필요할 것 같아요!
id랑 비밀번호 등을 저장하는 user data class인 줄 알았서요

hyeeum added 21 commits November 8, 2024 14:04

Feature #11: network setting

148c3d2

Chore #11: extract strings

eca37a7

Chore #11: feature package align

75519a4

Chore #11: feature package align

13cbd4f

Feature #11: di package

265203e

Feature #11: dto package

84616e6

Feature #11: entity package

b5b6e0f

Feature #11: WavveDataSource

084407f

Feature #11: WavveDataSourceImpl

41840c9

Feature #11: WavveRepository

6c47389

Feature #11: WavveRepositoryImpl

37d3db6

Feature #11: WavveService

6db3300

Chore #11: rename App to WavveApplication

a3176cf

Chore #11: change email,password to token

450ed5e

Feature #11: MyState

4538e9b

Feature #11: MyViewModel

db02568

Chore #11: delete signUpEmail,signUpPassword parameters

f68a47e

Chore #11: edit parameter email to username

9712368

Chore #11: edit parameter email to username and add hobby

52c7573

Chore #11: connect signup api

5748826

Chore #11: rename function,class name

765cb5f

hyeeum added the enhancement New feature or request label Nov 14, 2024

hyeeum requested review from HAJIEUN02, kamja0510 and 1971123-seongmin November 14, 2024 18:08

hyeeum self-assigned this Nov 14, 2024

greedy0110 reviewed Nov 16, 2024

View reviewed changes

kamja0510 approved these changes Nov 17, 2024

View reviewed changes

HAJIEUN02 approved these changes Nov 17, 2024

View reviewed changes

1971123-seongmin approved these changes Nov 17, 2024

View reviewed changes

jihyunniiii reviewed Nov 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Week4] 4주차 필수 과제 #12

[Week4] 4주차 필수 과제 #12

hyeeum commented Nov 14, 2024 •

edited

Loading

greedy0110 left a comment

greedy0110 Nov 16, 2024

greedy0110 Nov 16, 2024

greedy0110 Nov 16, 2024

kamja0510 left a comment

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

kamja0510 Nov 17, 2024

HAJIEUN02 left a comment

HAJIEUN02 Nov 17, 2024

HAJIEUN02 Nov 17, 2024

HAJIEUN02 Nov 17, 2024

HAJIEUN02 Nov 17, 2024

1971123-seongmin left a comment

1971123-seongmin Nov 17, 2024

1971123-seongmin Nov 17, 2024

1971123-seongmin Nov 17, 2024

1971123-seongmin Nov 17, 2024

1971123-seongmin Nov 17, 2024

1971123-seongmin Nov 17, 2024

jihyunniiii left a comment

jihyunniiii Nov 19, 2024

jihyunniiii Nov 19, 2024

jihyunniiii Nov 19, 2024

jihyunniiii Nov 19, 2024

jihyunniiii Nov 19, 2024

[Week4] 4주차 필수 과제 #12

Are you sure you want to change the base?

[Week4] 4주차 필수 과제 #12

Conversation

hyeeum commented Nov 14, 2024 • edited Loading

Related issue 🛠

Work Description ✏️

Screenshot 📸

To Reviewers 📢

greedy0110 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

kamja0510 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

HAJIEUN02 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

1971123-seongmin 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

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

hyeeum commented Nov 14, 2024 •

edited

Loading