[내 코드가 그렇게 이상한가요?] 1장 잘못된 구조의 문제 깨닫기

 

나쁜 구조로 인해 발생할 수 있는 폐해

 

1. 의미를 알 수 없는 이름

예시1) 기술 중심 명명

class MemoryStateManager{
	void changeIntValue01(int changeValue){
    	...
    }
}

 

-자료형(Int), 메모리 제어를 나타내는 Memory와 Flag 등 프로그래밍이나 컴퓨터 용어를 기반으로 이름을 붙이는 것을 기술 중심 명명이라고 부릅니다.

 

 

 

 

예시2) 일련번호 명명

class Class01{
	void method001();
	void method002();
    void method003();
    ...
}

 

- 클래스와 메서드에 번호를 붙여서 이름을 짓는 것을 일련번호 명명이라고 합니다.

 

- 이렇게 이름을 지으면 코드의 의도를 읽어낼 수 없고, 충분히 이해하지 못한 상태로 코드를 변경하면 버그가 발생합니다.

 

- 그래서 스프레드시트 등을 사용해 각 클래스와 메서드의 역할과 기능을 설명하는 문서를 만듭니다.

    -> 이런 문서는 바쁜 업무로 인해 유지 보수가 거의 이루어지지 않습니다.

 

 

 

[개선]

 의도와 목적을 드러내는 이름을 사용하는 것이 좋습니다. 이렇게만 해도 구조가 간단하고 명확해집니다.

 

 

 

 

 

2. 이해하기 어렵게 만드는 조건 분기 중첩

if(조건){
	//수십~수백 줄의 코드
    
    if(조건){
    	//수십~수백 줄의 코드
        
        if(조건){
            //수십~수백 줄의 코드
            
            if(조건){
                //수십~수백 줄의 코드
                
                if(조건){
                    //수십~수백 줄의 코드
                }
            }
        }
    }
}

 

if 조건문 내부에 if 조건문, 그리고 그 안에 또 if 조건문이 있는 상태를 if 조건문이 중첩되어 있다고 합니다.

 

 

[단점]

코드의 가독성이 나빠집니다.

    - 어디서부터 어디까지가 if 조건문 처리 블록인지 확인하기 힘듭니다.

    - 조건이 복잡해질수록 코드를 읽고 이해하기 힙듭니다.

 

 

 

 

 

 

 

3. 수많은 악마를 만들어 내는 데이터 클래스

 

 

 

설계를 따로 고려하지 않으면 아래 코드 처럼 데이터 클래스에 계산 로직을 추가하는 것이 아니라 다른 클래스에 구현하는 일이 발생합니다.

 

 

 

1) 데이터밖에 없는 클래스 구조 -> 데이터 클래스

//계약 금액
public class ContractAmount{
    public int amountIncludingTax; //세금 포함 금액
    public BigDecimal salesTaxRate; //소비세율
}

 

 

 

 

2) 세금이 포함된 금액을 계산하는 로직

public class ContractManager{
    public ContractAmount contractAmount;
    
    //세금 포함 금액 계산 로직
    public int calculateAmountIncludingTax(int amountExcludingTax, BigDecimal salesTaxRate){
    	...
    }
    
    //계약 체결 로직
    public void conclude(){
    	...
        int amountIncludingTax =
        	calculateAmountIncludingTax(amountExcludingTax, salesTaxRate);
        contractAmount = new ContractAmount();
        contractAmount.amountIncludingTax = amountIncludingTax;
        contractAmount.salesTaxRate = salesTaxRate;
        ...
    }
}

 

 

 

 

예) 업무 계약 서비스에서 소비세가  변경되었을 경우

 

데이터 클래스와 로직이 분산이 되어있어서 소비세와 관련된 부분을 소스 코드 전체에서 찾아야 합니다. 그럼 세금 포함 금액을 계산하는 로직이 수십 곳에 있는 것을 확인합니다. 

 

 

계산 로직을 어느 한곳에 만들어 두면, 사람들이 모두 그것만 따로 구현하지는 않겠지 하고 생각할 수도 있습니다. 하지만 설계에 관심이 없다면, 필요한 로직이 이미 구현되어 있다는 사실을 모르고 따로 구현해 버릴 수도 있습니다.

 

 

 

응집도가 낮을 때 발생하는 문제

 

1) 코드 중복

 - 관련된 코드가 서로 멀리 떨어져 있으면, 관련된 것끼리 파악하기 힘듭니다. 이미 구현이 되어있지만, 모르는 채 동료들이 같은 로직을 여러 곳에 구현할 수도 있습니다.

 

 

2) 수정 누락

 - 코드 중복이 많으면 사양이 변경될 때 중복된 코드를 모두 고쳐야 합니다. 

 

 

3) 가독성 저하

  - 코드가 분산되어 있으면, 중복된 코드를 포함해서 관련된 정보를 다 찾는 것만으로도 시간이 오래걸립니다. 따라서 가독성이 떨어지는 것입니다.

 

 

4) 초기화되지 않는 상태 (쓰레기 객체)

ContractAmount amount = new ContractAmount();
System.out.println(amount.salesTaxRate.toString());

 

 - 위 코드를 실행시키면 NullPointExcetpion이 발생합니다. salesTaxRatesms BigDecimal로 정의되어 있어서, 따로 초기화를 하지 않으면 null이 들어갑니다. ContractAmount가 추가로 초기화해야 하는 클래스라는 것을 모르면, 버그가 발생하기 쉬운 불완전한 클래스입니다.

 

 - '초기화하지 않으면 쓸모 없는 클래스' 또는 '초기화하지 않은 상태가 발생할 수 있는 클래스'를 안티 패턴 쓰레기 객체라고 부릅니다.

 

 

 

5) 잘못된 값 할당

ContractAmount amount = new ContractAmount();
amount.salesTaxRate = new BigDecimal("-0.1");

 

 - 소비세율을 음수로 대입해도 들어갑니다. 따라서 잘못된 값이 쉽게 들어갈 수 있는 구조입니다. 

 

 - 잘못된 값이 들어가지 않게, 데이터 클래스를 사용하는 쪽의 로직을 살짝 변경해서 유효성 검사를 하게 만들 수 있습니다.

    -> 하지만, 세금 포함 금액을 계산하는 로직처럼 여러 곳에 코드를 중복해서 유효성 검사를 해야하는 단점이 있습니다.