Week7 필수과제 구현 #12

sayyyho · 2024-12-17T14:59:29Z

Related issue 🛠

closed [Week7] 7주차 과제 구현 #11

Work Description ✏️

작업 내용

Screenshot 📸

이전 화면과 동일합니다.

Uncompleted Tasks 😅

각종 에러처리

To Reviewers 📢

MVI 진짜 쉽지 않네요...
이전 코리 반영과 더 고민해서 추가 작업해보겠습니다.
이번 주도 다들 시험 보느라 고생하셨어요!

imtaejugkim

State와 Effect 사용을 잘 하신것 같아요! 고생하셨습니다!

imtaejugkim · 2024-12-19T02:08:06Z

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

뭔가 가독성 측면에서, baseViewModel 과 interface, Util 파일을 분리해도 좋을 것 같아요!

imtaejugkim · 2024-12-19T02:19:57Z

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

+    private fun validateUserName(username: String) =
+        username.isNotBlank() && username.length <= 8
+
+    private fun validatePassword(password: String) =
+        password.isNotBlank() && password.length <= 8
+
+    private fun validateHobby(hobby: String) =
+        hobby.isNotBlank() && hobby.length <= 8
+}


이 부분은 서버에서 검증해주고 있어 필요 없을거 같네요!

imtaejugkim · 2024-12-19T02:21:40Z

app/src/main/java/org/sopt/and/presentation/signup/SignUpContract.kt

+        data object NavigateToSignIn : SignUpUiEffect()
+        data object NavigateUp : SignUpUiEffect()
+    }


오 저는 navigate이동과 popup을 동시에 사용했는데, 분리할 수도 있군요!

chattymin

저번에 코리 남겨달라 하셨어서 한번 놀러왔어요~

제가 일정이 있어서 전부 다 읽지는 못하고 위에 절반 정도밖에 못읽었어요.
그런데 잘하시네여 ㅎㅎ 잘 보고갑니당

질문하고 싶으신 코멘트가 있다면 멘션하고 답글 달아주셔도 좋고 따로 연락주셔도 좋습니다 :)

앱잼 화이팅입니다~ 🚀

chattymin · 2024-12-19T05:48:04Z

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

+    @Provides
+    @Singleton
+    fun provideUserRegisterRepository(userService: UserService): SignUpRepository {
+        return SignUpRepositoryImpl(userService)
+    }


@provides와 @BINDS의 차이점을 공부해보고 적용한다면 좋을 것 같아요!

그리고 DI에 대해서 조금 더 고민해보시면 좋을 것 같아요.
이미 inject를 통해 service를 받고있는데 여기서 또 매개변수를 넣어주고 있는 것 같네요

// 아래 코드에서 일부 발췌 class SignInRepositoryImpl @Inject constructor( private val userService: UserService ) : SignInRepository { ... }

이 코드만 말하는게 아니라 아래있는 다른 모듈들도 공통되는 사항이니 한번 확인 부탁해요!

chattymin · 2024-12-19T05:56:00Z

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

+            Pair(400, "01") to "잘못된 요청입니다.",
+            Pair(400, "01") to "아이디, 비밀번호, 취미를 올바르게 입력해주세요.",


사소한 실수~
key 두개가 똑같아용

chattymin · 2024-12-19T05:57:29Z

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

+    fun UserData.toUserLoginRequestDto(): SignInRequestDto = SignInRequestDto(
+        username = this.username,
+        password = this.password
+    )
+
+  fun UserData.toRegisterRequestDto(): SignUpRequestDto {
+        return SignUpRequestDto(
+            username = this.username,
+            password = this.password,
+            hobby = this.hobby
+        )
+    }
+
+}
+


두가지 방법이 혼용되어있는 것 같아요.
Mapper를 object로 사용하실거라면 내부에는 단순 function을 넣는게 일반적이고, 확장함수처럼 사용하신다면 굳이 Mapper로 감싸지 않아도 된답니다

chattymin · 2024-12-19T05:58:47Z

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

+    val result: MyHobbyResponseResultDto? = null,
+    val code: String? = null


serialName이 없어도 현재는 정상 작동될거에요
그런데 프로덕션으로 출시를 하고, 난독화 과정을 거친다면 result라는 변수는 a와 같이 변환되어요. 그렇기 때문에 난독화 한다면 서버통신 과정에서 오류가 발생할 수 있습니다

그렇기에 serialName을 붙여주는게 좋은 습관입니당

chattymin · 2024-12-19T06:03:47Z

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

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


저는 !! 연산자를 상당히 싫어하는 편이에요.
물론 현재는 runCatching으로 감싸두었기 때문에 여기서 오류가 난다고 해도 Result가 fail로 전달되어 앱은 안전할거에요.

하지만 오류를 확인하는 곳에서는 NPE로 오류가 나올것이기 때문에 오류를 찾기 힘들어질 수도 있어요. 그렇기에 null이 오류라면 throw를 활용해 명시적으로 알려주거나 null이 정상이라면 그것 자체를 반환하는 것도 방법이 될 수 있을 것 같아요

chattymin · 2024-12-19T06:08:30Z

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

+class MyHobbyUseCase(
+    private val getMyHobbyRepository: MyHobbyRepository


이부분도 Inject로 받아온다면 더욱 편해질거에요

어 뭐야 SignInUseCase에선 쓰셨네요?

chattymin · 2024-12-19T06:11:30Z

app/src/main/java/org/sopt/and/presentation/home/HomeContract.kt

+        val mainContents: List<HomeContent> = emptyList(),
+
+        val commonContents: List<HomeCommonContent> = emptyList(),


List는 불변객체가 아닌 읽기전용이에요. 그렇기에 리컴포지션의 대상이 돼요.
@stable, @immutable 어노테이션을 활용하거나 ImmutableLIst를 활용하신다면 리컴포지션을 줄일 수 있을거에요

chattymin · 2024-12-19T06:12:14Z

app/src/main/java/org/sopt/and/presentation/home/HomeScreen.kt

+    LazyColumn(
+        modifier = modifier, verticalArrangement = Arrangement.spacedBy(16.dp)
+    ) {


이렇게 활용하신다면 LazyColumn을 사용하는 의미가 없는 것 같아요.
Column에 ScrollState를 적용하는건 어떨까요??

chattymin · 2024-12-19T06:14:06Z

app/src/main/java/org/sopt/and/presentation/home/components/HomeBannerPager.kt

+    HorizontalPager(
+        state = pagerState,
+        contentPadding = PaddingValues(horizontal = 10.dp),
+        pageSpacing = 10.dp
+    ) { page ->
+        HomeBannerPage(
+            index = page,
+            banners = banners
+        )
+    }


Pager, Lazy~ 이런것들을 사용하실 때 key를 활용하시면 리컴포지션을 줄일 수 있을거에요

chattymin · 2024-12-19T06:15:33Z

app/src/main/java/org/sopt/and/presentation/home/components/Top20List.kt

+        Modifier.height(240.dp)
+    ) {
+        Image(
+            painter = painterResource(rankers[index]),


painter내부를 보면 UnStable해요. 그렇기에 Stable한 ImageVector를 사용한다면 리컴포지션을 줄일 수 있어요.

hyoeunjoo

매주 과제 수행하시느냐 너무 고생많으셨습니다!! 앱잼도 화이팅~

hyoeunjoo · 2024-12-19T06:15:55Z

app/src/main/java/org/sopt/and/core/component/HomeTobBar.kt

+@Composable
+fun HomeTopBar(
+    onLiveButtonClick: () -> Unit
+) {


home에서만 쓰는 topbar라면 home쪽으로 옮겨줘도 좋을 것 같아용
여기는 파일 전역에서 쓰는 컴포넌트들을 넣어주면 될 것 같습니다

hyoeunjoo · 2024-12-19T06:20:38Z

app/src/main/java/org/sopt/and/presentation/signup/SignUpUiState.kt

+data class SignUpUiState(
+    val signUpUsername: String = "",
+    val signUpPassword: String = "",
+    val signUpHobby: String = "",
+    val isSignUpPasswordVisible: Boolean = false
+)
+
+sealed class SignUpResult {
+    object Initial : SignUpResult()
+    object Success : SignUpResult()
+    object FailureInformationLength : SignUpResult()
+    object FailureDuplicateUsername : SignUpResult()
+}


Contract에 있는 UiState랑은 어떤게 다른건가요?? SignUpUiState가 두개 있길래요!

jihyunniiii

고생하셨습니다 ~

jihyunniiii · 2024-12-24T04:35:19Z

app/src/main/java/org/sopt/and/Route.kt

UI 단에서 사용되는 sealed class니 presentation 단에 위치시켜주어도 좋을 것 같아요!

jihyunniiii · 2024-12-24T04:35:47Z

app/src/main/java/org/sopt/and/core/ContentType.kt

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

jihyunniiii · 2024-12-24T04:36:47Z

app/src/main/java/org/sopt/and/core/navigation/Screen.kt

위에 있는 Route class와 같은 친구인 것 같은데 확인해보시면 좋을 것 같네요

jihyunniiii · 2024-12-24T04:37:26Z

app/src/main/java/org/sopt/and/core/utils/PreferenceUtil.kt

마찬가지로 core에 위치시키신 이유가 있는지 궁금합니다.
data를 저장하고 가져오는 로직이니 data 레이어와 조금 더 어울리는 것 같아서요.

jihyunniiii · 2024-12-24T04:39:59Z

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-24T04:43:51Z

app/src/main/java/org/sopt/and/presentation/main/MainBottomTab.kt

+import org.sopt.and.R
+import org.sopt.and.core.navigation.Screen
+
+data class MainBottomTab(


enum을 사용해보셔도 좋을 것 같네요 ~

jihyunniiii · 2024-12-24T04:45:46Z

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

+    getMyHobby: () -> Unit,
+    modifier: Modifier = Modifier
+) {
+    getMyHobby()


getMyHobby의 결과값을 넘겨주는 형태로 해서 MyInfoProfile의 책임을 더 분리하는 게 어떨까요?

jihyunniiii · 2024-12-24T04:46:56Z

app/src/main/java/org/sopt/and/presentation/signup/SignUpContract.kt

+        data object NavigateUp : SignUpUiEffect()
+    }
+
+    enum class Field {


enum으로 선언하신 이유가 있나요?

sayyyho added 16 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

chore #11 - hilt 사용 초기 세팅

4276b73

refactor #11 - SignInScreen MVI 리팩토링

c7c16cf

refactor #11 - SignUpScreen MVI 리팩토링

1b2f1c7

fix #11 - 실행 오류 해결

949ccfc

refactor #11 - MVI 리팩토링

2f88fef

sayyyho requested review from imtaejugkim, jihyunniiii and hyoeunjoo December 17, 2024 14:59

sayyyho linked an issue Dec 17, 2024 that may be closed by this pull request

[Week7] 7주차 과제 구현 #11

Open

1 task

imtaejugkim approved these changes Dec 19, 2024

View reviewed changes

chattymin reviewed Dec 19, 2024

View reviewed changes

hyoeunjoo approved these changes Dec 19, 2024

View reviewed changes

jihyunniiii reviewed Dec 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Week7 필수과제 구현 #12

Week7 필수과제 구현 #12

sayyyho commented Dec 17, 2024

imtaejugkim left a comment

imtaejugkim Dec 19, 2024

imtaejugkim Dec 19, 2024

imtaejugkim Dec 19, 2024

chattymin left a comment •

edited

Loading

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

hyoeunjoo left a comment

hyoeunjoo Dec 19, 2024

hyoeunjoo Dec 19, 2024

jihyunniiii left a comment

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

jihyunniiii Dec 24, 2024

		Pair(400, "01") to "잘못된 요청입니다.",
		Pair(400, "01") to "아이디, 비밀번호, 취미를 올바르게 입력해주세요.",

		val result: MyHobbyResponseResultDto? = null,
		val code: String? = null

		class MyHobbyUseCase(
		private val getMyHobbyRepository: MyHobbyRepository

		val mainContents: List<HomeContent> = emptyList(),

		val commonContents: List<HomeCommonContent> = emptyList(),

Week7 필수과제 구현 #12

Are you sure you want to change the base?

Week7 필수과제 구현 #12

Conversation

sayyyho commented Dec 17, 2024

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

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

chattymin left a comment • edited Loading

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chattymin left a comment •

edited

Loading