클린 코드 적용기 - 함수는 작게, 한가지 역할만

(※ 실제 코드 유출 방지를 위해 약간의 코드 변형을 하였습니다.)

함수를 작게 만들어라

• 함수를 만드는 규칙은 작게, 더 작게
• 함수 안에 들어가는 if-else 혹은 while은 한 번만 사용되도록 하자

함수는 한 가지 역할만 하자

• 함수는 한 가지만 하고, 그 한 가지를 잘 해야 한다

아래 코드는 사용자의 상태에 따라 UI에 그려줄 화면을 지정하는 함수이다.

Bad Practice

func checkViewState() {
    if let userData = AuthManager.sharedInstance.getUserData() {
        if userData.member {
            if let _ = userData.memberId {
                self.userData = userData
                viewState = .main(userData: userData)
            } else {
                viewState = .launch
            }
        } else {
            viewState = .verificationWait
        }
    } else {
        viewState = .intro
    }
}

문제점

1. 하나의 함수에 12줄의 코드로 구성되어있다. (12줄 정도는 통상적으로 이해할 수 있을지도 모른다.)
2. 더 큰 문제는 하나의 함수에 if-else문이 벌써 3번이나 사용되어 가독성을 해치고 있다. 이는 2가지 관점에서 굉장히 Bad Practice라고 할 수 있다. 첫째, 다른 팀원들은 이 코드를 이해하기 위한 시간과 노력이 적지 않게 소모된다. 둘째, 테스트 코드를 작성할 때에도 썩 좋지 않다. 즉, 네 가지 경우의 viewState를 테스트하기 위해 동일한 함수가 여러 번 테스트될 것이다.

Good Practice

func checkViewState() {
    if let userData = AuthManager.sharedInstance.getUserData() {
        setViewStateIfIsMember(userData)
    } else {
        viewState = .intro
    }
}

func setViewStateIfIsMember(_ userData: UserData) {
    if userData.member {
        setViewStateIfMemberIdExists(userData)
    } else {
        viewState = .verificationWait
    }
}

func setViewStateIfMemberIdExists(_ userData: UserData) {
    if let _ = userData.memberId {
        self.localData = localData
        viewState = .main(localData: localData)
    } else {
        viewState = .launch
    }
}

  
//  setViewStateIfMemberIdExists()에 대한 테스트 코드
  
func testSetViewStateIfMemberIdExists() {
    let localData = LocalData(accessToken: accessToken, accountId: accountId, zoneIds: zoneIds, activeZoneId: activeZoneId, member: true, memberId: 123, activeZoneName: activeZoneName, activeZoneType: activeZoneType, addressDetailACode: addressDetailACode, addressDetailAName: addressDetailAName, addressDetailB: addressDetailB, username: nil, apartmentResident: false, sendbirdAccessToken: nil, invalidInfo: nil, isWewootCrew: isWewootCrew)
    
    viewModel.setViewStateIfMemberIdExists(localData)
    let exp: WewootAppViewState = .main(localData: localData)
    XCTAssertEqual(viewModel.viewState, exp)
}

func testSetViewStateIfMemberIdNotExists() {
    let localData = LocalData(accessToken: accessToken, accountId: accountId, zoneIds: zoneIds, activeZoneId: activeZoneId, member: true, memberId: nil, activeZoneName: activeZoneName, activeZoneType: activeZoneType, addressDetailACode: addressDetailACode, addressDetailAName: addressDetailAName, addressDetailB: addressDetailB, username: nil, apartmentResident: false, sendbirdAccessToken: nil, invalidInfo: nil, isWewootCrew: isWewootCrew)
    
    viewModel.setViewStateIfMemberIdExists(localData)
    let exp: WewootAppViewState = .launch
    XCTAssertEqual(viewModel.viewState, exp)
}

변경점

1. 함수 당 if-else문은 한 번만 사용하여 하나의 조건(condition)을 검증하는 역할을 하도록 수정하였다.
2. 함수의 역할에 따른 함수명을 재정의하였다.
3. 함수 각각에 대한 개별적인 테스트 코드를 작성하였다.
4. '내려가기' 규칙을 사용하여 위에서 아래로 갈수록 함수의 추상화 수준이 한 단계씩 내려가도록 함수의 순서를 지정하였다.

회고

코드를 수정하기 전에는 '나한테는' 익숙했기 때문에 인식하지 못한 문제였지만, Bad Practice의 코드는 다른 팀원들과의 협업 및 유지보수 관점에서 좋지 못한 코드임을 느끼게 되었다.
함수를 작게 분리하는 것이 생각보다 개발 초보자들한테는 익숙하지 않은 것 같아서 꾸준히 작게, 더 작게 만드려는 노력을 해야겠다.