[iOS - swift] Clean Code(클린 코드) - 12. 냄새와 휴리스틱 (1) (주석, 환경, 함수, 일반)

* 냄새와 휴리스틱: 파틴 파울러의 "Refactoring"에서 "코드 냄새"를 표현하여 주의해야하는 코드 사항들에 대해 나열한 것에 더하여 저자 로버트 C.마틴의 생각을 더한 것

주석

• 쓸모없는 주석
  • 주석은 빨리  낡으므로 쓸모 없어진 주석은 `의식`하여 재빨리 없애야 코드를 그릇된 방향으로 이끌지 않게하므로 주의
• 중복된 주석
  • 코드 내용과 중복되는 주석 내용을 피할 것

WRONG

- 코드 내용과 중복되는 주석

i += 1 // i 증가

WRONG

- 메소드 시그니처만 달랑 기술하는 주석 (parameter의 내용만으로 충분히 의미가 표출되지만, 불필요한 주석)

    /**
     (summary 부분)
     > Description 부분
     - Parameters:
     - sellRequest: Sell을 하기위해 필요한 리퀘스트 파라미터
     
     */
    func beginSellItem(sellRequest: SellRequest) {
        print(sellRequest)
    }

cf) 주석 사용 방법: https://ios-development.tistory.com/650

• 주석 처리된 코드는 바로 제거하는 것을 지향
  • 코드를 읽다가 주석으로 처리된 코드가 줄줄이 나오면 이 코드가 얼마나 오래된 코드인지, 중요한 코드인지 알 길이 없으므로 주석 처리된 코드가 보이면 바로 삭제할 것

환경

• 빌드는 여러단계로 하지 않고 간단히 한 단계로 끝내는 것을 지향
  
  • 불가해한 명령이나 스크립트를 잇달아 실행하여 각 요소를 따로 빌드할 필요가 없어야하게끔 유지
  • 한 명령으로 전체를 체크아웃해서 한 명령으로 빌드되게끔 유지

WRONG

- fastlane을 통해 빌드를 실행할 때, 인증서 갱신 후 빌드를 실행 시키는 경우 여러 명령어를 일일이 입력

$ fastlane match development --readonly
$ fastlane build_app(...)

RIGHT

- .sh파일을 만들어서 따로 있던 명령어들을 적어넣고 사용할땐 .sh로 실행

$ ./build_app.sh

함수

• 인수의 갯수
  • 함수에서 인수 개수는 작을수록 좋으므로 넷 이상의 인수가 생겨날 경우 따로 struct로 만들어서 데이터형을 하나로 변경할 것
• 플래그 인수
  • Bool 인수는 함수가 여러 기능을 수행한다는 명백한 증거이므로, SRP를 위반하게 되므로 bool을 피할 것

일반

• 모든 테스트를 통과하게끔 설계
  
  • 스스로의 직관에 의존하지 말고 모든 구석진 곳, interface에 해당하는 부분들을 테스트하는 테스트 코드를 작성할 것
• 인터페이스에는 최소한의 함수만을 표출하도록 설계
  
  • 부실하게 정의된 모듈은 인터페이스가 구질구질한 형태
  • 장 정의된 인터페이스는 많은 함수를 제공하지 않으므로 결합도(coupling)이 낮은 형태
  • 자료를 숨기고, 유틸리티 함수, 상수, 임시적으로 사용되는 변수를 숨김으로서 정보를 매우 작게 낮추어 결합도를 낮추는 형태
• Bool을 인수로 갖는 함수를 지양
  • Bool을 인수로 갖는다는 의미는 함수 안에서 true or false에 관한 분기문 처리가 들어가므로, 하나의 함수에서 두 가지의 일을 처리하게 되는 현상

WRONG

ex) 주급을 계산하는 함수이고, 초과근무 수당을 지급하면 1.5배, 지급하지 않으면 1.0배의 계산을 수행하는 코드

- 인수로 isOverTime을 받고 있는데, 이 값은 곧 함수 안에서 분기문이 생기고 두 가지의 일을 처리하게 되는 형태 (SRP 원칙 위반)

func calculateWeeklyPay(isOverTime: Bool) {
    let tenthRate = getTenthRate()
    let tenthsWorked = getTenthsWorked()
    let straightTime = min(400, tenthsWorked)
    
    let overTime = max(0, tenthsWorked - straightTime)
    let straightPay = straightTime * tenthRate
    let overtimeRate = overtime ? (1.5 * tnethRate) : (1.0 * tnethRate)
    let overtimePay = round(overTime * overtimeRate)
    
    return straightPay + overtimePay
}

RIGHT

- Bool 인수값을 제외하고 다른 time값을 받도록 구현

- 함수 안이 짧은 형태

func straightPay() -> Int {
    return getTenthsWorked() * getTenthRate()
}

func overTimePay() -> Int {
    let overTimeTenths = max(0, getTenthsWorked() - 400)
    let overTimePay = overTimeBonus(overTimeTenths)
    return straightPay() + overTimePay
}

private func overTimeBonus(_ overTimeTenths: Int) {
    let bonus = 0.5 * getTenthRate() * overTimeTenths
    return round(Int(bonus)!)
}

• 코드의 배치 위치는 함수 이름들, 클래스 이름들을 보고, 자기 자신의 기준이 아닌 처음보는 개발자가 보아도 "당연히 이곳에 있는 곳"에 위치할 것

ex) PI 상수는 Math 클래스, Trigonomery 클래스, Circle 클래스 중 어느 곳에? -> 삼각함수를 선언한 클래스에 위치

• static은 override할 가능성이 없고 객체에서 가져오는 정보가 없는 경우에 사용

WRONG

- 특정 객체와 관련이 없으면서 모든 정보를 인수에서 가져오지만, 함수를 override할 가능성이 존재하므로 안좋은 코드

- override가능성 예시) OVertimeHourlyPayCalculator, StraightTimeHourlyPayCalculator

HourlyPayCalculator.calculatePay(employee, overtimeRate)

RIGHT

- 특정 객체와 관련이 없으며 모든 정보를 인수에서 가져오고 함수를 override할 가능성이 없는 코드

- 일반적으로는 static 함수보다 인스턴스 함수가 더 좋으므로 애매한것이라면 인스턴스 함수로 사용할 것

Math.math(a: Double, b: Double)

• 가독성 향상을 위해 서술적인 변수를 자주 사용하여, 하나의 계산을 여러 단계로 쪼갤것

WRONG

- 해당 코드를 봤을 때 match.group(1)와 match.group(2)의 의미를 파악할 수 없는 상태

let match = headerPattern.matcher(line)

if match.find() {
    headers.put(match.group(1), match.group(2))
}

RIGHT

- 한번의 계산을 쪼개어서 서술적인 이름을 붙인 변수로 표현하여, 그룹1이 key값이고 그룹2가 value값인것을 파악할 수 있는 상태

if match.find() {
    let key = match.group(1)
    let value = match.group(2)
    headers.put(key, value)
}

* 참고: Clean Code (로버트 C. 마틴)