Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat/#8] 4주차 과제 #9

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open

[Feat/#8] 4주차 과제 #9

wants to merge 21 commits into from

Conversation

jm991014
Copy link
Contributor

@jm991014 jm991014 commented Nov 14, 2024

Related issue 🛠

Work Description ✏️

  • 3주차 코드리뷰 반영 (안된게 있을 수도 있어요..)
  • 필수
    • 유저 등록 API 연동
      • 8자 초과로는 입력하지 못하게 제한 및 regex로 validation 진행
      • 등록 시 실패하는 경우가 중복된 아이디일 경우밖에 없는듯 하여 Toast로 메시지 출력
    • 로그인 API 연동
      • 유저 등록과 동일하게 8자 초과로는 입력 제한
      • 로그인 실패는 기존과 동일하게 유지
    • 사용자 취미 조회 연동
      • 마이페이지 진입 시 내 취미 받아오도록 설정
  • 심화
    • 다른 사람 취미 조회
      • 기존 뷰 지우기가 너무 아까워서.. 스낵바로 띄웠습니다.
    • 내 정보 변경(취미 / 비밀번호)
      • 취미만 입력될 경우, 비밀번호만 입력될 경우를 나눠서 각각
        1. 취미: bottomsheet를 닫고 내 취미 새로고침
        2. 비밀번호: 변경 시 로그아웃하여 재로그인 하도록 설정
        3. 만약 두 필드 각각 제한 사항에 맞지 않으면 응답코드가 200이지만 실제 서버에는 반영이 되지 않던데 해당 부분을 앱단에서 제한하려 했으나 투머치 같아서 넘어갔습니다!
      • botoomsheet가 좀 구리긴 한데..ㅎㅎ

Screenshot 📸

�필수 심화(정보변경) 심화(취미조회)
signup.mp4
user.mp4
search.mp4

Uncompleted Tasks 😅

  • 4주차 도전과제: Debounce 및 로딩 view
  • 3주차 심화과제: Typography
  • 3주차 도전과제: 애니메이션 적용
  • 2주차 심화과제: 검색 view (horizontal pager)
  • 2주차 도전과제: Room 적용

To Reviewers 📢

심화 과제를 진행하던 중에 정보 변경 시 200일 때는 빈 응답값을 받게 되어 에러가 발생하더라구요... 그래서 해당 부분을

@PUT("/user")
    suspend fun modifyMyHobby(
        @Body hobbyModifyRequest: HobbyModifyRequest
    ): Response<BaseResponse<String?>?>

이처럼 Response로 감싸주어 해결하긴 했는데 다들 더 좋은 의견이 있다면 공유해주시면 감사할 것 같습니다!

@jm991014 jm991014 self-assigned this Nov 14, 2024
@jm991014 jm991014 added the enhancement New feature or request label Nov 14, 2024
Copy link

@kangyein9892 kangyein9892 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저와는 다른 아이디어가 많은 코드였던 것 같아요 👍👍

저와는 다른 코드 구조라서 맞는 방법일지는 모르겠지만... 그냥 함수의 반환타입? 값?이 없이 진행하면 안되려나요..!?

@PUT("/user")
    suspend fun modifyMyHobby(
        @Body hobbyModifyRequest: HobbyModifyRequest
    )

요렇게요...!!

import javax.inject.Inject


class TokenInterceptor @Inject constructor(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sharedPreference에 token이 저장되어있으면 header에 추가해주는건가요?
방법이 있는 건 알았지만 머리가 잘 안 굴러가서 결국 시도를 못했는데...!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 있다는 것만 알고 이번에 처음 적용해봤습니다ㅎㅎ 좋더라구요

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우왕 하나 배워갑니다~!

data class SignUpResponse(
@SerialName("no") val userId: Int
) {
fun toId() = UserId(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

진짜 사소한거고 개인적인 생각이지만... 뭔가 그냥 toId라고 하니까 id랑 헷갈려서 userId라고 다 붙여주던가 뭔가 no number? 이런 네이밍을 가져가도 좋을 것 같다는 생각이 듭니다...!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이게.. 의식의 흐름대로 하다보니 이 부분만 좀 네이밍이 구린거 같아요.. 수정해두겠습니다!

import org.sopt.and.domain.repository.MyPageRepository
import javax.inject.Inject

class GetHobbyUseCase @Inject constructor(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 내 취미조회랑 타인 취미조회랑 합쳐서 usecase로 한거 아이디어 좋네요 !
단순히 Empty보다 의미있는 상수를 넣어서 진행해도 좋을 것 같아요!

Copy link

@boiledEgg-s boiledEgg-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제 눈엔 완벽하네요💯 너무 고생많으셨습니다!!

import javax.inject.Inject


class LocalDataSourceImpl @Inject constructor(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localdl라는 이름으론 이 클래스가 어떤 저장소에 접근하는지 알기 어렵다고 생각해요.
로컬 데이터베이스의 경우 preference, datastore, room 등이 있으니까 이름에서 잘 구분해주면 좋을 것 같습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

힐트 사용 너무 잘하시네요~

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 네이밍을 왜 앱 모듈로 하셨나요?
내용과 이름이 전혀 다르다고 생각되네요,,

Comment on lines +12 to +15
fun toUserHobby() = UserHobby(
hobby = hobby,
password = ""
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 매핑을 위한 함수는 하나의 패키지에 모아두는 편입니다!
개인취향이니까 뭐가 더 편할지 생각해보세요~

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

무슨 오류가 발생했는지 쉽게 알아볼 수 있는 방법이네요!!
저도 이렇게 관리해봐야겠네요ㅎㅎ

Copy link

@serioushyeon serioushyeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드가 완전 깔끔하네요 고생하셨습니다~~!!

@Provides
@Singleton
fun provideLoggingInterceptor(): HttpLoggingInterceptor = HttpLoggingInterceptor().apply {
level = HttpLoggingInterceptor.Level.BODY

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HttpLoggingInterceptor 사용하는거 너무 좋은 것 같아요~!!

import javax.inject.Inject


class TokenInterceptor @Inject constructor(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우왕 하나 배워갑니다~!

data class HobbyResponse(
@SerialName("hobby") val hobby: String
) {
fun toUserHobby() = UserHobby(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 내부에 넣을 수도 있군요 저와 다른 방법이라 신기하네용!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 4주차 과제
4 participants