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

Feature/enroll view model #79

Merged
merged 26 commits into from
Dec 6, 2022
Merged

Feature/enroll view model #79

merged 26 commits into from
Dec 6, 2022

Conversation

jinwoong16
Copy link
Collaborator

📕 Issue Number

Close #77

📙 작업 내역

구현 내용 및 작업 했던 내역

  • EnrollViewModel이 모든 bind와 관련되게 변경
    • 이제 EnrollViewModel이 모든 bind의 input과 output을 조정합니다.
    • 이로인해 기존의 서브 viewModel이었던 DayNamePickerViewModel은 삭제되었습니다.
  • EnrollView에 버튼 추가
  • EnrollView에서 시작과 끝날, 그리고 요일을 입력받으면, 해당하는 모든 날짜들을 계산 (viewmodel이 합니다.)

Screenshot 2022-12-05 at 6 04 45 PM

📘 작업 유형

  • 신규 기능 추가
  • 버그 수정
  • 리펙토링
  • 문서 업데이트

📋 체크리스트

  • Merge 하는 브랜치가 올바른가?
  • 코딩컨벤션을 준수하는가?
  • PR과 관련없는 변경사항이 없는가?
  • 내 코드에 대한 자기 검토가 되었는가?
  • 변경사항이 효과적이거나 동작이 작동한다는 것을 보증하는 테스트를 추가하였는가?
  • 새로운 테스트와 기존의 테스트가 변경사항에 대해 만족하는가?

📝 PR 특이 사항

PR을 볼 때 주의깊게 봐야하거나 말하고 싶은 점

  • 예상했던대로, 코드수가 확실히 늘어났습니다... 시작과 끝날, 그리고 요일을 입력받으면 모든 날짜를 계산해주는 로직(getDates(start:end:weekday:))을 usecase단으로 옮길 생각을 하고 있습니다.



@@ -17,10 +20,11 @@ final class QuantityView: UIView {
return titleLabel
}()

private lazy var quantityField: UITextField = {
private(set) lazy var quantityField: UITextField = {
Copy link
Member

Choose a reason for hiding this comment

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

아까 말씀하신 방법으로 바꾸신 거 확인했습니다!

input.quantityDidSet) { title, quantity in
!title.isEmpty && !quantity.isEmpty
}
.asDriver(onErrorJustReturn: false)
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
Collaborator Author

Choose a reason for hiding this comment

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

리팩터때, 시작날과 끝나는날도 저쪽부분에 추가해서 핸들링해야할 것 같아요.
끝나는날이 시작하는날보다 이르면 안되니까..!

@Jeonhui
Copy link
Member

Jeonhui commented Dec 5, 2022

combineLatest같은 결합 연산자를 되게 잘 사용하셨네요. 저도 더 열심히 공부해봐야겠습니다.
이제 프로젝트도 얼마 안 남았는데 조금만 더 같이 힘내요😊

@jinwoong16 jinwoong16 added this to the 5주차 개발! milestone Dec 5, 2022
Copy link
Collaborator

@wickedRun wickedRun left a comment

Choose a reason for hiding this comment

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

고생하셨습니다. 잘봤습니당

var date = start

while date <= end {
guard let weekday = Calendar(identifier: .gregorian).dateComponents([.weekday], from: date).weekday else { return nil }
Copy link
Collaborator

Choose a reason for hiding this comment

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

이부분은 extension 합쳐지면 더 짧게 고쳐질 것 같긴합니당

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

refactor 주간에 고쳐봐야겠습니다 :)

import Foundation

import RxSwift
import RxCocoa
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 Driver 때문이지요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 맞습니다. driverRxCocoa에 구현되어있어서요. 하나하나 전부 스케줄러를 지정해서 쓰는것보단 있는거 가져다 쓰는게 좋을 것 같아서요 ㅋㅋㅋ

@jinwoong16 jinwoong16 merged commit 9469d49 into develop Dec 6, 2022
@jinwoong16 jinwoong16 deleted the feature/EnrollViewModel branch December 6, 2022 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EnrollView 퀘스트 등록 로직
3 participants