-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Refactor] 2주차 과제 MVVM, 에러핸들링, UX 개선 #11
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 :)
protocol SignInViewModel: SignInViewModelInput, SignInViewModelOutput { } | ||
|
||
final class DefaultSignInViewModel: SignInViewModel { | ||
|
||
private var email: String | ||
private var password: String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 프로젝트에서는 데이터가 email 따로 password따로라서 이게 가능하지는 않겠지만 코멘트를 남겨보면
이번에 석우님은 의존성? 관련해서 코드를 리팩토링하시는거같아서 관련해서 제가 알고있는 부분을 남기겠습니다
저도 이부분은 아직잘모르지만 이런 방법도 쓸수는있겠구나 정도로만 생각하며 읽어주면 고맙겠습니다
우선 email, password를 하나의 구조체로 묶는다고 한다면( 보통 데이터를 entitiy라는 구조체로 관리를 하니까요)
protocol BaseViewModel {
associatedtype T
func fetchData()
func getDate() -> [T]
func addData(_ data: T)
func deleteData(index: Int)
}
관리를 해야할 데이터타입을 제네릭으로 선언해주고(protocol에서 generic은 associatedtype를 통해서 관리하죠)
이 데이터에 따라 로직을 관리할수있을거같기는해요
제 코드에선 input output을 따로 프로토콜로 나누지는 않았는데 석우님 코드처럼 나눠도 괜찮을거같아요
석우님이
func emailTextFieldDidChangeEvent(_ text: String)
func passwordTextFieldDidChangeEvent(_ text: String)
라고 하신부분을 사실지금은 text가 string이 확실하니까 제가 리뷰님긴 부분처럼 할필요는 전혀 없지만
나중에 하나의 데이터 타입을 가지고 사용할일이 생긴다면 제네릭으로 표현해서 모든 viewmodel에 사용할 baseViewModel프로토콜을 만들때 도움이 될거같아요
그런 범용적인 baseViewModel을 만들고 모든 viewModel에 이 baseViewModel을 채택하게되면
그 Protocol을 기준으로 모든 ViewController에 ViewModel의 의존성을 주입시킬수있을거같아요
typealias BaseViewController<T> = ViewController<T> where T: BaseViewModel
BaseViewController를 이런식으로 타입선언을해주고
이걸바탕으로 BaseViewController를 만들면
class ViewController<T>: UIViewController {
let viewModel: T
init(viewModel: T) {
self.viewModel = viewModel
super.init(nibName: nil, bundle: nil)
}
required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}
override func viewDidLoad() {
super.viewDidLoad()
view.backgroundColor = .red
setUI()
setConstraints()
}
func setUI() {}
func setConstraints() {}
}
나중에 이 ViewController를 채택하는 곳에서 ViewModel의 의존성을 넣어줄수있을거같아요(이때 ViewModel은 BaseViewModel을 채택한거겠죠)
class MainViewController: BaseViewController<MainViewModel> {
private var testLabel: UILabel = UILabel().then {
$0.text = "testLabel"
$0.font = .systemFont(ofSize: 15)
$0.tintColor = .blue
$0.textAlignment = .center
}
override func viewDidLoad() {
super.viewDidLoad()
// 주입된 viewModel사용가능
viewModel.deleteData(index: 1)
}
override func setUI() {
view.addSubview(testLabel)
}
override func setConstraints() {
testLabel.snp.makeConstraints { make in
make.center.equalToSuperview()
make.size.equalTo(100)
}
}
}
이번에 석우님이 mvvm으로 리펙토링하신다고해서 제가 mvvm으로 baseVC나 baseVM을 만들때 고려했던 부분들을 남겼습니다!
읽어보시고 그냥 흘리셔도 괜찮아요 ㅋㅋㅋㅋㅋㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제네릭을 통해서 의존성 주입해주는 방법 좋네요 !
나중에 하나의 데이터 타입을 가지고 사용할일이 생긴다면 제네릭으로 표현해서 모든 viewmodel에 사용할 baseViewModel프로토콜을 만들때 도움이 될거같아요
확실히 이런 상황에서 정말 유용하게 쓸 수 있을 것 같네요! 저번주 덥덥디씨 스터디 주제랑 겹쳐서 더 흥미롭게 읽었습니다ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
믿고보는 맛집👍 리팩 깔끔하네요!
protocol AuthTextFieldDelegate: AnyObject { | ||
func authTextFieldTextDidChange(_ textFieldType: AuthTextField.TextFieldType, text: String) | ||
func authTextFieldDidReturn(_ textFieldType: AuthTextField.TextFieldType) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여러개의 클로저보단 delegate가 역시 깔끔하네요!👍
override func viewWillAppear(_ animated: Bool) { | ||
super.viewWillAppear(animated) | ||
|
||
emailTextField.becomeFirstResponder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UX개선점 좋네요!
enum AuthError: Error { | ||
case invalidEmail | ||
case invlidPassword | ||
case invalidUser | ||
|
||
var message: String { | ||
switch self { | ||
|
||
case .invalidEmail: | ||
return "이메일형식을 잘못 입력하셨습니다." | ||
case .invlidPassword: | ||
return "비밀번호를 8자 이상 입력해주세요." | ||
case .invalidUser: | ||
return "존재하지 않는 회원입니다." | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 에러모델 만들어주고
|
||
protocol SignInViewModelOutput { | ||
var ableToSignIn: Observable<Bool> { get } | ||
var isSuccessLogin: Observable<Result<Bool,AuthError>> { get } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 이렇게 넘겨주는거 깔끔하네요 배워갑니다👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오와~ 저도 배워가요 ❤️
|
||
import Foundation | ||
|
||
final class Observable<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove시에 동일한 인스턴스 시에 제거할려고 class쓰신것 같네요! 여기서 struct로 바꾸기 위해서 제가 1차 과제 때 uuid를 활용해서 리팩한거 있는데 한번 참고해보시고 의견 알려주시면 감사하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uuid를 사용하면서 struct로 구현하시는 이유는 무엇인가용?
순환참조될 가능성 때문인가엽?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct과 class의 성능 차이때문입니다 ! 그 둘을 구분짓는 가장 큰 차이점이 value semantics냐 reference semantics냐인데 class를 사용하게 되면 reference semantics를 사용하게 되겠죠. class는 heap에 데이터를 저장하기 때문에 reference counting과 더불어서 메서드를 dynamic하게 dispatch하기 때문에 성능면에서 struct에 비해 상대적으로 떨어집니다 ! 실제로 이러한 이유로 Swift에서 class가 필요한 상황이 아니라면 struct을 권장합니다.
Observable 클래스는 상속할, 상속받아야할 객체가 아니기에 class보다 struct으로 바꾸는 것이 더 낫다고 생각해서 저는 struct으로 구현했습니다. 둘다 상관없다면 struct이 맞는 방향이라고 생각합니다 ! class여야만 한다고 생각하시는 이유가 혹시 있으실까요?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 그냥 다른 플젝에서 쇽샥해온거라 Class
로 할지 struct
로 할지 크게 고민해보지 않은 것 같아욥..
semantics 처음 들어보는데 공부좀 해봐야겠네욥 감사합니당 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨어요~
target() | ||
binding() | ||
delegate() | ||
bind() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 최근에 알게 된 건데, init() 할 때 ViewModel로 bind 해주는게 viewDidLoad에서 하는것 보다 클린하다고 하네요.
근데 의견차이가 있다고 합니다..
bind 함수 성격상 저도 init() 에 있는게 어울린다고 생각해요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정완!
|
||
import Foundation | ||
|
||
enum AuthError: Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 enum이나 struct와 같은 Model은 presentation layer(ViewModel)에서 정의 되어있기 보다는 domain layer 쪽으로 넘겨서 사용하는게 클린에 어울린다고 하더라고요!
Usecase로 로직처리를 따로 하고 있진 않지만 이후 클린 아키텍쳐로 리팩토링할 때 참고하면 좋을것 같아요!
|
||
protocol SignInViewModelOutput { | ||
var ableToSignIn: Observable<Bool> { get } | ||
var isSuccessLogin: Observable<Result<Bool,AuthError>> { get } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오와~ 저도 배워가요 ❤️
//MARK: - Output | ||
|
||
var ableToSignIn: Observable<Bool> = Observable(false) | ||
var isSuccessLogin: Observable<Result<Bool,AuthError>> = Observable(.failure(.invalidEmail)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 이런식으로 넘길수도 있군요!
//MARK: - Init | ||
|
||
init(email: String, password: String) { | ||
self.email = email |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emailTextFieldDidChangeEvent에서 이미 self.email = text
초기화 진행되는데 init에서 추가로 초기화 받은 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DefaultSignInViewModel는 Class 로 선언하였고
email 과 password는 String타입으로만만 선언해주어서 init에서 초기화 하지 않으면 Class initializer가 필요하다구 해서 넣었습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하~ signInViewModel이 생성되는 부분에서 email, password 모두 "" (빈문자열) 로 초기화 되고 있어서
init으로 초기화 하지 않고 옵셔널 처리하는것도 방법이 될 수 있겠네용~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스에서 어떤게 좋은 초기화 방법일꽈,,,
- 프로퍼티 타입 선언만 하고 init에서 초기화 지정
- 프로퍼티 타입 선언과 동시에 빈 문자열로 초깃값 지정
- 프로퍼티를 옵셔널로 선언
나는 1을 선호하긴 하는데 @hongjunehuke 이는 어떤걸 선호하니
@kimscastle 킹캐슬님은 어떤거 선호하세연?
@ffalswo2 만재형은 어떤거 선호하세연?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 ViewController init으로 의미있는 값을 넘긴다면 init을 선호하고 아니라면 3번 선호하는것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1번 같은 경우는 보통 객체등을 의존성 주입할 때, 그리고 2번은 초기값이 필요한 경우에, 마지막 3번은 초기값 조차 없고 나중에 설정되는 값일 때 주로 쓰는 것 같아요. 적고보니까 너무 뻔한 말 같네요??😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 3개모두 특정한 상황에서 딱딱 역할이 있는거 같다고 생각하긴해요
근데 제가 처음에 프로젝트를 할때 3번만 썼다가 진짜 guard문 지옥에 떨어졌던적이있는데
그때 제 코드를 본 한 팀원이 정말 값이 없을수있는 경우만 3번을 쓰라고 했었어서
그때이후로는 정말 옵셔널이 가능한 상태일때만 3번을쓰고
1번이나 2번을 자주쓰기는 하는거같긴해요
1번 2번중에서는 2번대신 1번 init에서 초기화 지정할때 input의 기본값을 지정하는걸 더 선호하는거같아요
특별하게는 didSet을 쓸때는 3번을 사용해서 값을 잘 받아오는지 guard문으로 체크하는 용도로 쓰기는합니다(이건 저의 습관인거같아요)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow 정말 생산적인 답변 감사합니다 여러분 ..🥹
갈증이 해소된것 같아욥..!!
🔥 Pull requests
✅ 작업한 내용
SignInVC 리팩토링 진행했습니다.
emailTextField
에서Return
버튼 누를시 ->passTextField
키보드 활성화passwordTextField
에서Return
버튼 누를시 ->SignInButtonDidTap
함수 호출❗️PR Point
해당 리팩토링하며 꽤 많은 정보가 들어와서 뇌에 과부화가 온 상황입니다.
🕍 아키텍처
MVVM과 클린아키텍처를 도입하려고 합니다.
MVVM은 구현한 것 같으나 클린아키텍처는 완벽히 구현하지 못하였습니다..
클린 아키텍처를 이해하기 위해선
객체지향적
,의존성
개념을 잘 알아야 할 것을 같아요(객체지향이 의존성인가?)객체지향은 어느정도 익숙하지만
의존성
개념이 부족하여의존성
을 중점적으로 공부해보려구용의존성 개념이 익숙해지면 MVVM + 클린아키텍처로 구현해야겠어요
🎨 디자인패턴
Observable 패턴을 도입했어요.
View에서 VM에서 값이 바뀌는 것을 관찰(observe)하기 위해서요!
에러핸들링을 위해 Result<Data,Error> 타입을 사용했어요.
이부분이 조금 섹시한 것 같습니다 개인적으로
🍀 UX개선
becomFirstResponder()
view.endEditing(true)
활용하여 UX 개선했습니다.
@seongmin221 님이 에러메시지 위쪽에 있는게 좋을 것 같다는 코드리뷰를 참고하여 상단에 에러메세지 띄웠어요.
🦴 ViewModel 코드 뼈대 설명
📸 스크린샷
RPReplay_Final1682630827.mov
👀 레퍼런스
참고 프로젝트
⭐️⭐️<MVVM + 클린아키텍처 깃허브>
<소복소복>
<SOPT 공식앱>
클린 아키텍처 이해
<클린아키텍처 창시자 마틴형 원본>
⭐️⭐️<클린 아키틱처 - zeddiOS>
<클린 아키텍처 실제 적용사례 - 우아한형제들>
<의존성 주입, DIP>
그 외
<Result 타입>
<키보드제어>
시험 끝나서 행복하네요
closed #10