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] #43 - 회원가입 뷰 Domain, Data 구현 #52

Merged

Conversation

lsj8706
Copy link
Member

@lsj8706 lsj8706 commented Dec 20, 2022

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 회원가입 뷰 Domain, Data 레이어 구현했습니다.
  • network 부분 연결하여 정상적으로 작동하는 것을 확인했습니다.

📌 참고 사항

  • SignUpRepository에서 2 종류의 service가 필요해서 추가로 생성하고 모듈팰토리에서 주입했습니다.
  • 통신 후 받아오는 response가 없어서 requestObjectInCombineNoResult 사용했습니다.
  • 모델과 엔티티는 request를 위해 만들었습니다.
  • 현재 아키택쳐로 목데이터가 아니라 실제 서버 연결까지 해본 것은 처음이라 이 방식이 맞는지 확인 부탁드립니다..!
  • 서버 통신할 때 로딩 뷰 같은 게 없어서 조금 어색한 느낌이 있습니다... 에러 처리도 없어서 나중에 다듬는 과정이 필요할 것 같아요!

📸 스크린샷

기능 스크린샷
회원가입 뷰 서버 통신 Simulator Screen Recording - iPhone 13 mini - 2022-12-20 at 18 47 00

📮 관련 이슈

@lsj8706 lsj8706 added Feat 새로운 기능 구현 세진🤝🏻 labels Dec 20, 2022
@lsj8706 lsj8706 self-assigned this Dec 20, 2022
Copy link
Contributor

@L-j-h-c L-j-h-c left a comment

Choose a reason for hiding this comment

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

역시 깔끔하네요 수고하셨습니다! 그리고 회원가입 로그인 결과의 userId를 UserDefault에 저장해야 할 것 같은데 이부분은 어떻게 될까요??

Comment on lines 23 to 26
requestObjectInCombineNoResult(.signUp(nickname: signUpEntity.nickname,
email: signUpEntity.email,
password: signUpEntity.password)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

일단 저희가 초기에 논의한 바로는, entity는 서버의 response라고 생각하시면 됩니다! 그리고 Data Layer의 transform에서 entity->Model로 바꿔주고, model로 바꾸었으니 이를 비즈니스 로직에서 사용할 수 있게 됩니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

파라미터로 쪼개는 방식으로 수정했습니다! 감사합니다 ㅎㅎ


public func signUp(signUpModel: SignUpModel) {
repository.postSignUp(signUpModel: signUpModel)
.map { statusCode in statusCode == 200 }
Copy link
Contributor

Choose a reason for hiding this comment

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

요런 부분은 레포지토리에서 처리하고, 유즈케이스는 레포지토리에 보낸 요청이 성공했는지 실패했는지에 대해 판단할 수 있을 것 같은데 요건 지극히 제 개인적인 생각이니 참고만 해주세용~~

Comment on lines 28 to 40
public func getNicknameAvailable(nickname: String) -> AnyPublisher<Int, Error> {
return networkService.getNicknameAvailable(nickname: nickname)
}

public func getEmailAvailable(email: String) -> AnyPublisher<Int, Error> {
return networkService.getEmailAvailable(email: email)
}

public func postSignUp(signUpModel: SignUpModel) -> AnyPublisher<Int, Error> {
return userService.postSignUp(signUpEntity: SignUpEntity(nickname: signUpModel.nickname,
email: signUpModel.email,
password: signUpModel.password))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

지금은 로직이 간단해서 이렇게도 가능하지만, 나중에 복잡해지면 repository에서 service의 결과에 대한 판단을 해줄 수 있을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다..! 혹시 이렇게 바꾼 게 맞을까요?

Copy link
Member

@yangsubinn yangsubinn left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ! 🙇‍♀️

Comment on lines 36 to 39
case .getNicknameAvailable:
return .get
case .getEmailAvailable:
return .get
Copy link
Member

Choose a reason for hiding this comment

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

리턴값이 같은 케이스는 같이 묶어서 관리해두 좋을 것 같아여 !

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다!

output.signUpSuccessed
.sink { [weak self] isSuccess in
guard let self = self else { return }
isSuccess ? self.presentSignUpCompleteView() : print("회원가입 실패")
Copy link
Member

Choose a reason for hiding this comment

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

회원가입 실패할 경우, 사용자가 볼 수 있는 alert를 띄워주는건 어떤가여?

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 머지하고 추후에 워딩이 확정되면 다른 서버 통신하는 부분들까지 한번에 alert를 추가하겠습니다!

Copy link
Member

@devxsby devxsby left a comment

Choose a reason for hiding this comment

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

고생하셨습니당
참고좀할게요.?

@lsj8706 lsj8706 merged commit c70cbf2 into sopt-makers:develop Dec 22, 2022
@lsj8706 lsj8706 deleted the feat/#43-회원가입-Domain-Data-구현 branch December 22, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feat 새로운 기능 구현 size/L 세진🤝🏻
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 회원 가입 뷰 Domain 및 Data 구현
4 participants