반응형
본 포스팅은 코드리뷰를 진행하며 팀원으로부터 받은 피드백과 클린코드 도서를 종합하여 새롭게 꺠달은 점이나 코드를 고쳐나가는 과정을 기록하는 포스팅입니다.
혹시 틀린 정보가 있다면 댓글로 알려주시면 감사하겠습니다 :)
1. 함수는 작으면 작을수록 좋다.
하나의 블록 안에 코드가 길어지면 길어질수록 이해하기 힘들다. 길어지는 부분을 또 하나의 기능 단위로 나누어 함수화하고, 이를 호출하는 것이 더 명확하게 이해하기 쉬워진다.
2. if - else 블록은 가급적이면 뎁스 1개가 가장 좋다.
블록 안에 if - else 문이 또 들어가게 되면 함수 가독성이 떨어지게 된다.
팀원의 말을 가져오면 차라리 만족하지 않는 조건일 경우에 return 시켜버리는게 더 좋은 방법이라고 한다. (if 블록 뎁스가 2개 이상을 넘어가면 읽기가 너무 어려워지므로)
*Early Return에 관한 reference: https://velog.io/@jangws/9.-Else-if%EC%99%80-Else%EB%A5%BC-%ED%94%BC%ED%95%98%EA%B3%A0-Early-Return%ED%95%98%EC%9E%90
[클린코드 JS] Else if와 Else를 피하고 Early Return하자
else if문이 마치 파이프라인처럼 앞의 if문과 연결되어 차례대로 실행된다고 생각하면 안 된다.else if문은 else문 처리가 한 번 되고 if문 동작이 되는 것과 같다.위와 아래의 코드는 논리적으로 같
velog.io
if !authInsertInfoView.phoneNumberTF.isEmpty {
if !MatcherUtils.isRightLengthDigit(length: 10, digit: authInsertInfoView.phoneNumberTF.getText) {
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_notten
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
} else { // 10자리 이상이면: 유효한 휴대폰번호인지 체크하고
if !MatcherUtils.isPhoneNumber(phoneNumber: authInsertInfoView.phoneNumberTF.getText) {
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_onlynumber
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
} else { // 유효하면 검정 + 에러 없어지기
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 0.46, green: 0.46, blue: 0.46, alpha: 1)
authInsertInfoView.phoneGuideLabel.isHidden = true
authInsertInfoView.isAvailableCheckList[2] = true
}
}
} else {
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_placeholder
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
}
해당 코드는 if - else 뎁스가 3개까지 들어가므로 가독성이 좋지 않다. 이를 조건 3개로 나누어 early-return 처리하여 적용하면 다음과 같이 수정할 수 있다.
if authInsertInfoView.phoneNumberTF.isEmpty { // 휴대폰 번호가 비어있는 경우
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_placeholder
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
return
}
if !MatcherUtils.isRightLengthDigit(length: 10, digit: authInsertInfoView.phoneNumberTF.getText) { // 10자리 미만 입력하면
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_notten
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
return
}
// 10자리 이상이면: 유효한 휴대폰번호인지 체크
if !MatcherUtils.isPhoneNumber(phoneNumber: authInsertInfoView.phoneNumberTF.getText) {
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_onlynumber
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
} else { // 유효하면 검정 + 에러 없어지기
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 0.46, green: 0.46, blue: 0.46, alpha: 1)
authInsertInfoView.phoneGuideLabel.isHidden = true
authInsertInfoView.isAvailableCheckList[2] = true
}
이부분을 좀 더 swift스럽게 바꾼다면, guard 문을 사용하여 early return을 적용할 수 있을 것이다.
guard !authInsertInfoView.phoneNumberTF.isEmpty else { // 휴대폰 번호가 비어있는 경우
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_placeholder
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
return
}
guard MatcherUtils.isRightLengthDigit(length: 10, digit: authInsertInfoView.phoneNumberTF.getText) else { // 10자리 미만 입력하면
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_notten
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
return
}
// 10자리 이상이면: 유효한 휴대폰번호인지 체크
if !MatcherUtils.isPhoneNumber(phoneNumber: authInsertInfoView.phoneNumberTF.getText) {
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 1, green: 0.28, blue: 0.28, alpha: 1)
authInsertInfoView.phoneGuideLabel.text = Strings.SignUpV2_hometax_auth_phone_guide_onlynumber
authInsertInfoView.phoneGuideLabel.isHidden = false
authInsertInfoView.isAvailableCheckList[2] = false
} else { // 유효하면 검정 + 에러 없어지기
authInsertInfoView.phoneNumberTF.layer.borderColor = CGColor(red: 0.46, green: 0.46, blue: 0.46, alpha: 1)
authInsertInfoView.phoneGuideLabel.isHidden = true
authInsertInfoView.isAvailableCheckList[2] = true
}
3. 변수는 의미있는 이름으로 지어야한다.
개발을 같이 하는 동료 모두가 이 변수가 어떤 의미를 지니는 변수인지를 알 수 있어야한다.
예를들어, 다음과 같은 코드를 보자.
(안좋은 코드)
func selectTab(tab: Int) {
switch tab {
case 0:
// 탭 설정 코드
case 1:
// 탭 설정 코드
(개선한 코드)
enum TabType: Int { // 탭의 유형을 열거형으로 정의
case home
case healthCare
case proxyHistory
case myPage
}
....
func selectTab(tabType: TabType) {
switch tab {
case .home:
case .mypage:
....
}
}
selectTab(tabType: .home)
4. 코드의 가독성을 위해 변하지 않는 변수는 let, 변할 수 있는 변수는 var로 선언하자. (성능은 크게 차이 없다고 함)
반응형