-
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
Feature/15 #22
Feature/15 #22
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.
コメントしました!よろしくお願いします!!
self?.presenter.onError(error: err) | ||
} | ||
|
||
if let uid = result?.user.uid { |
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.
if let
での処理ですが,else以降の処理がないので個人的には
guard let uid = result?.user.uid else {
self?.presenter.onError()
return
}
let userReference = Firestore.firestore().collection("message").document("v1").collection("users").document(uid)
とした方がネストが1つ少なくなってエラー処理もで切ると思うのですがどうでしょうか?
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.
確かにそちらの方が綺麗ですね、修正します!
if let error = error { | ||
Auth.auth().signIn(withEmail: email, password: password) { [weak self] (result, error) in | ||
if let err = error { | ||
self?.presenter.onError(error: err) |
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.
self?.presenter.onError(error: err)
の後に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.
忘れてました、こちらも修正します!
修正しました! |
概要
レビュー観点
レビューレベル
Lv0: まったく見ないでAcceptするLv1: ぱっとみて違和感がないかチェックしてAcceptするLv3: 実際に環境で動作確認したうえでAcceptする備考