부스트코스 “iOS 프로그래밍” 실습 프로젝트B. SignUp - 코드 리뷰
- Ref: 부스트코스 - iOS 프로그래밍
리뷰어에게 남긴 메모
프로젝트를 진행하면서 궁금했던 내용을 메모에 질문으로 남겼다.
- 클래스 이름을 정할 때 단축어를 사용하나요?
- 예) ViewControlelr -> VC
- 디버깅 로그
- 디버깅 로그를 최대한 꼼꼼히 남기는것이 좋을까요?
- 꼭 필요한 곳에만 남겨야 한다면 판단하는 기준은 무엇인가요?
- 특정 양식에 맞춰서 남기는지? 그 예시를 알고싶습니다.
리뷰어가 보내온 답변
안녕하세요. 보내주신 프로젝트 잘 확인해보았습니다.
프로젝트 진행하시느라 수고많으셨습니다. 많은 고민을 하신 것이 코드에 잘 드러나서 리뷰하면서 많이 배워갑니다.
질문 답변 드립니다.
- 거의 그렇게 사용하지 않습니다.
Clarity is more important than brevity. (명료함이 간결함보다 더 중요하다.)
Avoid abbreviations. (약어 사용을 피해라.)
Swift API 가이드라인에 명시된 내용이며, 협업 시에도 중요한 부분이니 참고해주세요~
- 디버깅 로그
코드 리뷰에도 코멘트를 남겼으니, 확인해주시기 바랍니다.
디버깅 로그는 프로젝트의 성격과 디버깅 로그를 남기는 방식에 따라 다르게 볼 수 있어서
딱 어떻게 하라고 말씀드리기 어려운 부분이 있는데요.
print로 찍는 것에 한정해서 말씀드리면, 가급적이면 최소화하는 것이 좋고 다음과 같은 판단 기준은 어떨까 싶습니다.
1. 앱 동작하면서 눈으로 확인할 수 없는 상태 변화 (Cache가 날라간다던지)
2. 크래시 상황에서 유의미한 정보를 남기려는 경우
개인적으로 특정 양식을 따르고 있지는 않습니다. 필요한 경우 호출된 context 파악을 위해 #file, #function, #line, #column 키워드를 활용하기도 합니다.
SignInViewController.swift
1. 간단한 기능은 Target-Action 패턴으로
텍스트 필드 외 영역을 탭 하면 키보드가 내려가는 기능을 강의에서 Target-Action 패턴으로 학습했다. 실습 프로젝트의 해당 뷰 컨트롤러에서도 추가 기능 없이 간단한 동작이므로 배운 대로 구현할까 하다가 델리게이트 패턴을 굳이 사용했더니 코드 리뷰를 남겨주셨다.
리뷰 코멘트
#AdviceDelegate를 이용하여 TapGesture를 처리해주셨네요. 이 방법도 옳은 방법이지만 TapGesture와 같이 간단한 경우에는
Target-Action 패턴을 활용하는 경우가 더 많습니다. 조금 더 간결해지기도 하구요.
한번 참고해보시면 좋을 것 같습니다.
https://developer.apple.com/documentation/uikit/uigesturerecognizer/1624230-addtarget
리뷰 적용
델리게이트 구현 내용은 주석 처리하고 Target-Action 패턴 내용을 추가했다.
// Target-Action 패턴으로 구현
extension SignInViewController {
@IBAction func tappedView(_ sender: UITapGestureRecognizer) {
view.endEditing(true)
}
func setupGestureRecognizer() {
let tapGesture: UITapGestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(self.tappedView(_:)))
self.view.addGestureRecognizer(tapGesture)
}
}
SignUpViewController.swift
1. 옵셔널 체크
옵셔널 체크만 하면 돼서 guard else 구문을 사용하고, 상수 이름을 ‘_‘로 생략했다.
var isValidProfileImage: Bool {
guard let _ = profileImageView.image else {
print("profileImage == nil")
return false
}
return true
}
리뷰 코멘트
#Advice
let _ = profileImageView.image 보다는 profileImageView.image != nil 이 좀 더 코드의 의미를 잘 드러내는 것 같습니다.
리뷰 적용
var isValidProfileImage: Bool {
guard profileImageView.image != nil else {
print("profileImage == nil")
return false
}
return true
}
2. Responder
평가 기준에는 없는 내용이지만 텍스트 필드 델리게이트 기능을 살펴보다가 추가한 기능이다.
ID 입력 후 return 키를 누르면 키보드가 내려가는데 이때 자동으로 다음 텍스트 필드로 커서가 이동한다면 사용자가 직접 텍스트 필드를 찾아갈 수고가 적어질 것이다.
텍스트 필드 델리게이트의 textFieldShouldReturn()은 텍스트 필드가 return돼도 되는지 묻는 메서드다. 즉, 리턴키를 누르면 호출된다.
이 메서드에서 각 텍스트 필드에서 입력을 한 후 return 키를 누르면 다음 텍스트 필드가 FirstResponder가 되도록 했다.
func textFieldShouldReturn(_ textField: UITextField) -> Bool {
// 리턴키 누르면 다음 텍스트 필드로 키보드 입력 커서 이동
switch textField {
case idTextField:
passwordTextField.becomeFirstResponder()
case passwordTextField:
checkPasswordTextField.becomeFirstResponder()
case checkPasswordTextField:
// 비밀번호 확인 텍스트필드 입력 후에는 텍스트 뷰로 커서 이동
descriptionTextView.becomeFirstResponder()
default: break
}
return true
}
리뷰 코멘트
#Excellent
사용자 경험을 생각하여 디테일한 부분까지 구현해주셨네요. 잘하셨습니다 :D
Responder 개념에 대해서도 잘 이해하고 계신 것 같아요.
3. 내비게이션 컨트롤러로 다른 뷰 컨트롤러 푸쉬
extension SignUpViewController {
@IBAction func touchUpNextButton(_ sender: UIButton) {
let signUpAddViewController = storyboard?.instantiateViewController(withIdentifier: "SignUpAddViewController") as! SignUpAddViewController
navigationController?.pushViewController(signUpAddViewController, animated: true)
}
}
리뷰 코멘트 #Advice
“SignUpAddViewController”로 하드코딩하는 대신 ViewController의 static property로 정의하는 것이 더 안전합니다.이렇게 하면 Identifier가 여러 군데에서 사용될 때 실수할 확률이 줄어들고, 나중에 수정할 때에도 편리합니다.
그리고 되도록이면 강제 언래핑은 피해주세요
리뷰 적용
SignUpAddViewController 클래스에 연산 프로퍼티로 “SignUpAddViewController” String을 반환하게 하려다가 더 좋은 방법을 검색했다.
class 키워드를 붙인 연산 타입프로퍼티로 클래스명을 반환하게 했다.
참고 - Clean Code for Multiple Storyboards
extension SignUpAddViewController {
/// 스토리보드 ID
/// - 연산 타입프로퍼티에 class 키워드 사용
class var storyboardID: String {
return "\(self)"
}
}
extension SignUpViewController {
@IBAction func touchUpNextButton(_ sender: UIButton) {
// 다음 화면으로 이동
guard let signUpAddViewController = storyboard?.instantiateViewController(withIdentifier: SignUpAddViewController.storyboardID) as? SignUpAddViewController else {
return
}
navigationController?.pushViewController(signUpAddViewController, animated: true)
}
}
이 방법은 스토리보드 ID를 클래스 이름과 동일하게 사용할 수 도, 클래스 이름 뒤에 “_ID”등의 추가 문자를 붙이더라도 “(self)” + “_ID” 등으로 수정하며 사용할 수 있다.
또한 강제 다운캐스팅을 사용하지 않고, gueard else구문으로 옵셔널 언래핑 후 사용하도록 수정했다.
SignUpAddViewController.swift
1. Computed Property를 이용하여 validation 로직 분리
휴대폰 번호 입력 상태를 확인하는 로직을 Computed Property로 분리했다.
var isValidPhoneNumber: Bool {
guard let phoneNumber = phoneNumberTextField.text else {
print("PhoneNumber == nil")
return false
}
guard phoneNumber.count >= 3 else {
print("PhoneNumber 3자리 이상 입력되지 않음")
return false
}
return true
}
회원 가입 기능의 특성상 조건을 확인하고 에러 처리하는 부분이 많았고, print()를 사용해서 로그를 남겼는데, 실무에서는 이런 로그를 어떤 식으로 처리하는지 궁금해서 리뷰어에게 질문을 남겼었다.
리뷰 코멘트 #Advice
Computed Property를 이용하여 validation 로직을 분리해주셨네요. 잘하셨습니다.
질문 주신 것중에 debug log에 대한 것이 있었는데요.
debug log는 말 그대로 디버깅할 때 도움이 될만한 정보를 기록하는 것입니다.
그런데 body안에서 false로 리턴되는 것들은 버그라기 보다는 validation에 실패한 에러라고 보는 게 더 타당합니다.따라서 조건 충족에 실패한 각 케이스들에 대해 뭔가 처리를 하고 싶다면
var isValidPhoneNumber를 func validatePhoneNumber() throws 로 만들어 조건에 충족하지 못하는 상황에 대해 각각의 에러를 throw하고
함수 바깥에서 throw된 에러를 받아 Alert을 띄우거나 간단하게는 print를 하는 식으로 처리할 수 있을 것 같습니다.개인적으로 너무 많은 print는 사실 디버깅할 때 방해가 되는 경우도 있기 때문에 Critical한 상황이 아니면 지양하는 편입니다.
Critical한 부분은 아니기에 따로 수정은 하지 않고 다음 프로젝트 시 Critical한 부분이 나온다면 코멘트 내용대로 에러 처리를 해야겠다.
2. 코드 한 줄로 역할이 충분히 설명된다면 굳이 한 줄 짜리 함수로 만들지 말자
싱글턴 클래스의 메서드를 사용하는데 굳이 따로 함수를 만들었다.
func clearUserInformation() {
UserInformation.shared.clear()
}
리뷰 코멘트 #Advice
한 줄 짜리 함수에서 그 한 줄로 함수의 역할이 충분히 설명되기 때문에 굳이 함수로 만들 필요는 없어보입니다.
리뷰 적용
여러곳에서 사용하는 기능도 아니고, 코드로 충분히 설명이 되기에 함수를 없애고 싱글턴 메서드를 그대로 사용하게 수정했다.
UserInformation.swift
1. 더 이상 상속이 필요없는 class는 final 키워드로 최적화 하자
리뷰 코멘트 #Advice
더 이상 상속이 필요없는 class의 경우 final 키워드를 붙여서 최적화할 수 있습니다.
Swift에서는 override를 위해 class function 호출 시 dynamic dispatch 테크닉을 사용합니다.
final 키워드를 붙이면 dynamic dispatch를 생략하므로 성능 향상에 도움이 됩니다.
https://developer.apple.com/swift/blog/?id=27
리뷰 적용
final class UserInformation {
...
}
2. 옵셔널 프로퍼티
유저정보 싱글턴 클래스의 프로퍼티를 옵셔널로 하는게 맞는지 질문을 남겼다.
// 질문: 아래와 같은 용도의 프로퍼티의 경우 옵셔널 변수로 하는게 맞는지 궁금합니다
// 기본정보
var id: String?
var password: String?
var description: String?
var profileImage: UIImage?
// 부가정보
var phoneNumber: String?
var birthdate: String?
리뷰 코멘트 #Advice
네 회원가입 과정 중에 아직 입력하지 않은 값들은 nil로 보는 것이 자연스러우므로
Optional로 지정하면 되겠습니다.
Leave a comment