Conversation
- KakaoOnboardingScreen 생성 (신체정보 수기입력) - authAPI에 submitOnboarding 함수 추가 (/api/users/onboarding) - LoginScreen에서 isOnboarded 값 확인하여 라우팅 분기 처리 - AppNavigator에 KakaoOnboarding 화면 추가
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <Stack.Screen name="KakaoOnboarding" component={KakaoOnboardingScreen} /> | ||
| {/* Main Tabs */} | ||
| <Stack.Screen name={ROUTES.MAIN} component={MainTabs} /> | ||
| {/* Payment Stack*/} |
There was a problem hiding this comment.
코드 패치에 대한 몇 가지 우려 사항이 있습니다.
-
확장성 및 유지 관리:
KakaoOnboardingScreen을 추가했지만, 해당 스크린이 의도한 대로 작동하는지에 대한 테스트가 필요합니다. 스크린이 연결된 라우트 이름이 하드코딩되어 있으므로, 변경 시 코드 전체를 수정해야 할 수 있습니다. 상수를 사용하는 것이 더 안전할 수 있습니다. -
내비게이션 순서:
KakaoOnboardingScreen이MainTabs이전에 추가되고 순서가 맞는지 확인해야 합니다. 만약 이 스크린이 사용자 흐름에 필수적이라면, 초기 화면으로 설정되어야 할 수도 있습니다. -
타입 체크 및 PropTypes: 만약 TypeScript를 사용하지 않고 있다면, 새로 추가된 컴포넌트에 대한 PropTypes를 정의하는 것이 좋습니다. 이는 런타임 오류를 줄이고 코드의 안정성을 높입니다.
전반적으로 코드 추가는 필요한 작업으로 보이지만, 위의 지적 사항들을 고려하여 테스트 및 검증을 거친 후 머지하는 것이 좋습니다.
| }); | ||
|
|
||
| export default KakaoOnboardingScreen; | ||
|
|
There was a problem hiding this comment.
코드 리뷰 코멘트
-
상태 관리:
formData의 각 속성에서 길이 검사를 할 때, 잘못된 값이 있는 경우 사용자에게 피드백을 제공하는 것은 좋으나 초기 값이 비어있는 경우 생성되는 오류는 필드가 비어 있음을 정확히 알리지 못할 수 있습니다. -
유효성 검사:
validateForm메서드에서 기본적인 유효성 검사를 하고 있지만, 모든 숫자 입력을Number()로 변환하기 전에 유효한 숫자 형식인지 확인하는 추가 검사가 필요할 수 있습니다. 예를 들어, 사용자가 문자 입력을 시도할 경우를 대비할 수 있습니다. -
온보딩 제출:
handleSubmit함수에서 API 호출에 대한 에러 처리가 있지만, 서버에서 제공하는 에러의 구조가 예상과 다를 경우에 대한 처리가 없습니다. 이런 부분은 전역 에러 핸들링을 고려하는 것이 좋습니다. -
모달 관리: 각 모달이 사용하는 상태 변수가 독립적이긴 하지만, 관리가 더 용이하도록 모달 상태를 하나의 객체로 통합하는 방법을 고려해 볼 수 있습니다. 이는 코드의 가독성을 높이고 상태 관리를 용이하게 할 것입니다.
-
직접적인 DOM 접근: Picker의 선택 값을 변경할 때
setTimeout을 사용하여 다음 렌더에 값을 수정하는 방식은 가독성을 저하시킬 수 있습니다.useEffect를 활용하여 상태 변경 후 특정 작업을 수행하도록 할 수 있습니다. -
TextInput: 키패드 입력 시 숫자 전용으로 제한하여 사용자 경험을 향상시킬 필요가 있습니다. 또한, 사용자에게 키를 입력할 때 실수를 방지하기 위한 적절한 안내를 추가하는 것이 좋습니다.
-
초기 값 설정:
tempPickerValue의 초기값에 대한 처리를 좀 더 명확하게 관리하는 방법을 고려해 보세요. -
리팩토링 가능성: 비슷한 코드는 함수로 추출하는 것을 고려하여, 중복을 줄이고 관리하기 쉽게 만드는 것이 좋습니다. 예를 들어, 모달 오프닝 및 클로징은 공통 함수로 만드는 것이 좋습니다.
이러한 개선점들에도 불구하고, 기본적인 구조는 잘 설계되어 있습니다. 하지만 몇 가지 수정이 필요해 보입니다.
| navigation.replace('KakaoOnboarding'); | ||
| } else { | ||
| navigation.replace('Main'); | ||
| } |
There was a problem hiding this comment.
코드 리뷰
잠재적 버그 및 위험:
-
비교 오류:
shouldOnboard의 판단 기준이isOnboarded === 'false' || onboarded === 'false'입니다. 이 경우 문자열 'false'와 실제 booleanfalse를 혼동할 수 있습니다. 타입 비교를 주의해야 합니다. -
회귀 테스트:
onboarded와isOnboarded의 비교가 모두 'false'일 때, 이전보다 더 복잡한 조건을 사용하게 되었으므로, 이로 인해 기존의 특정 로그인 시나리오에서 발생할 수 있는 예외를 검토해야 합니다. -
변동성 데이터 처리: API 응답에서
isOnboarded또는onboarded가 예상치 못한 형식(예: null 또는 undefined)이 올 경우, 이를 적절하게 처리하지 않으면 예외가 발생할 수 있습니다.
개선 제안:
- 명확한 타입 체크: 타입스크립트를 사용하는 경우 boolean 데이터 및 문자열 데이터 타입을 명확히 확인하고,
shouldOnboard변수의 조건문을 개선해주세요. - 로깅 개선: 로그인 실패 시 로깅 메시지를 더 구체화하여, 개발과 디버깅에 유리한 정보를 제공할 수 있습니다.
- 사용자 피드백: 사용자에게 보여줄 알림 메시지를 더 친절하게 작성하여, 오류 발생 시 사용자 경험을 향상할 수 있습니다.
결론:
위의 코드는 합리적인 변화를 포함하고 있지만, 추가적인 타입 체크 및 예외 처리가 필요합니다. 이를 개선하지 않으면 잠재적인 버그 및 사용자 경험 저하의 요인이 될 수 있습니다.
| throw error; | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
코드 패치를 검토한 결과, 몇 가지 우려 사항과 개선점을 발견했습니다.
-
타입 일관성:
userId와 같은 값이 백엔드 응답에서 비어 있을 경우, 예외적인 처리가 필요한 경우가 있습니다. 이 추가적인 예외 처리가 없으면userId가 undefined일 경우 불필요한 AsyncStorage 사용을 피하기 어렵습니다.response.userId를 비어 있는지 확인하고, 필요 시 기본값을 설정하는 것이 좋습니다. -
예외 처리: 모든 요청에서
catch블록이any타입의 오류를 사용하고 있기 때문에, 구체적인 에러 메시지를 로그로 출력하는 것이 유용할 수 있습니다. 이를 통해 디버깅이 용이해질 것입니다. -
JWT 페이로드 유효성 검사:
decodeJWT후에payload가 있는지 확인하는 것 외에도,userPk와 같은 중요한 필드가 절대적으로 필요한 경우, 더 확실한 유효성 검사가 필요합니다. 현재 로직에서는userId가 설정되어 있지 않은 상황에서만payload.userPk를 사용하는데, 이 경우 잘못된userId나userPk에 대한 명확한 처리가 필요합니다. -
하드코딩된 기본값:
membershipType에 대해 기본값 'FREE'을 항상 설정하고 있는데, 이 결정이 도메인 로직에 적합한지 고민할 필요가 있습니다. 만약 API에서 이 값을 제공하지 않는 경우에 대해서도 명확한 비즈니스 로직이 필요합니다. -
함수 분리:
kakaoLogin함수가 여러 가지의 책임을 가지게 되는 경향이 있습니다. 특히 AsyncStorage 작업과 API 요청 로직을 분리해서 관리하는 것이 유지보수 측면에서 더 유리할 수 있습니다. -
타입 정의: 타입 정의에 대해 좀 더 명확하게 하는 것이 좋습니다.
accessToken,refreshToken등을 포함하는 응답 데이터의 형태를 명확히 하고, 이에 대해 문서를 첨부하는 것도 좋은 방법입니다.
이러한 사항들을 개선한 후에는 머지를 위해 재검토하는 것이 좋겠습니다.
1 participant
No description provided.