리팩터링 2판 (코드에서 나는 악취)


"Hello world"를 처음 출력하고, Java로 코드를 작성할 무렵부터 "더 나은 코드는 어떤 것일까?" 라는 고민을 하였습니다.
하지만 다른 언어들과 프레임워크, 데이터베이스, CS 등 새로운 것들을 공부하느라 이러한 고민을 미뤄왔습니다.
아마 다른 주니어 개발자 분들도 공감할 것이라 생각합니다 😂
아직 갈길이 먼 신입 개발자이지만 좋은 개발자기 되기 위해 숙제로 미뤄왔던 '더 나은 코드'에 대한 고민을 다시 꺼내보고 싶습니다.
그래서 이 고민에 대한 답을 찾기 위한 첫걸음으로 리팩터링을 주제로 글을 작성하게 되었습니다. (마침 사내 스터디도 리팩터링...)

코드의 양을 최소화하며 의도를 명확하게 전달하기 위해 최대한 간단하고 직관적인 예제를 작성했습니다.
또한 실제적으로 리팩터링을 적용할 수 있는 것을 목표로 하였습니다. 고급(?) 리팩터링 기법보다는 실질적으로 당장 내일(?)부터 실천할 수 있도록 3장(Code Smell)의 내용을 집중적으로 다루었습니다. 개발을 공부하는 학생분들이나 리팩터링을 처음 접하시는 분들이 가볍게 읽고 도움이 되셨으면 합니다ㅎㅎ

참고 :
http://www.yes24.com/Product/Goods/89649360 (리팩터링 2판)
https://inf.run/t3o9 (백기선님 리팩터링 강의)


모호한 이름

"함수, 모듈, 변수, 클래스 등은 그 이름만 보고도 각각이 무슨 일을 하고 어떻게 사용해야 하는지 명확히 알 수 있도록 엄청나게 신경 써서 이름을 지어야 한다."

이름을 바꾸는 것으로 리팩터링을 시작하는 것이 어떨까요?
변수명이나 함수명을 정하는 것은 간단하지만 쉽지 않은 고민이라 생각합니다. Camel Case, Pascal Case, Snake Case 등 기본적인 규칙은 존재하지만 명확한 정답이 존재하지 않기 때문입니다. 간결하면서 의도를 명확히 나타낼 수 있는 이름을 정하기 위해 고려해야할 요소들은 여러가지 입니다.
장황하게 늘어진 이름은 가독성을 낮추고, 너무 간결한 이름은 의도를 온전히 전달하기 어렵습니다. 또한 기존 소스에서나 팀 내에서 관습적으로 사용되는 명명 스타일도 고려해야합니다.

public Boolean isApple(){
	if (...) return true;	
    else return false;
}

다소 극단적인(?) 예시 이지만, isApple() 라는 함수가 어떤 기능인지 예상이 가시나요? 클래스의 위치나 문맥 등을 본다면 파악할 수 있겠지만 함수명만 봐서는 사과인지, 애플의 기기인지 확신할 수 없습니다.


// 데이터를 가져오는 함수들
public SalesData loadSalesData(){...}

public AccountData loadAccountData(){...}

public CustomerData getCustomerData(){...}

모두 데이터를 가져오는 함수들이지만 getCustomerData() 를 보고 이질감이 느껴집니다. 이 함수는 왜 suffix가 왜 get이지??.. 뭔가 다른 기능인가?? 하는 의구심이 생깁니다.

// 데이터를 가져오는 함수들
public SalesData loadSalesData(){...}

public AccountData loadAccountData(){...}

public CustomerData loadCustomerData(){...}

이렇게 통일한다면 함수의 기능을 더 쉽게 예상할 수 있습니다.


// JWT 토큰에서 이메일 정보를 추출하여 반환하는 함수
private String convertJwtToEmail(DecodedJWT decodedJwt){...}

convert A to B 형식으로 함수명을 지정하여도 의미를 파악하는데는 어려움이 없지만 좀 더 간결하게 바꿔보겠습니다.

private String exractEamil(DecodedJWT decodedJwt){...}

함수 이름은 더 간결하면서도 의미 또한 명확하게 전달할 수 있습니다.
파라미터에 추출 대상에 대한 정보가 명시되어 있기 때문에 이를 활용하는 것도 함수명을 줄일 수 있는 방법입니다.


중복된 코드
"똑같은 코드 구조가 여러 곳에서 반복된다면 하나로 통합하여 더 나은 프로그램을 만들 수 있다."

중복되는 코드를 하나의 함수로 추출하는 것은 리팩터링의 기초라 할 수 있습니다. 코드의 양이 늘어나 가독성이 떨어진다는 1차적인 문제도 있지만, 중복 코드를 방치하는 것은 나중에 더 큰 문제를 야기할 수 있습니다.
만약 중복된 모든 곳을 수정하지 않고 일부만 수정한다면 이는 추후 예상치 못한 에러를 발생시킬 것 입니다.
또한 변수를 바꿔야하는 경우 일일히 중복코드들을 찾아 바꿔줘야 합니다. 이는 생산성 저하로 이뤄집니다.
중복되는 코드를 제거하는 방법으로는 함수로 추출하기와 상속구조를 이용하는 등 여러가지가 있습니다.

// before
public class BankService{

	// 입금
	public Account deposit(Customer customer, Account account, ...){
    	final String owner = account.getOwner();
        final String customerName = customer.getName();
        ...
        if(owner.equals(customerName)){...}
        else{...}
        
        Account result = new Account();
        result.setBalance(...);
        result.setDealDate(...);
        result.setBankLocation(...);
        ...
        
        return result;
    }
    
    // 출금
    public Account withdraw(Customer customer, Account account, ...){
    	final String owner = account.getOwner();
        final String customerName = customer.getName();
        ...
        if(owner.equals(customerName)){...}
        else{...}
        
        Account result = new Account();
        result.setBalance(...);
        result.setDealDate(...);
        result.setBankLocation(...);
        ...
        
        return result;
    }
}

입금, 출금 비즈니스 로직을 제외하고 계좌의 명의와 본인이 일치하는지 확인하는 코드와 거래 결과를 셋팅하는 부분이 반복됩니다. 다른 기능들에서도 반복될 가능성이 높기 때문에 별도의 함수로 분리합니다.

// after
public class AccountService{
	
    // 입금
	public Account deposit(Customer customer, Account account, ...){
        if(isOwner(account, customer){...}
        else{...}
        
        Account result = this.setResult(params....);
        
        return result;
    }
    // 출금
	public Account deposit(Customer customer, Account account, ...){
        if(isOwner(account, customer){...}
        else{...}
        
        Account result = this.setResult(params....);
        
        return result;
    }
    
    private Boolean isOwner(Account account, Customer customer){
    	return account.getOwner.equals(customer.getName);
    }
    
    private Account setResult(params...){
    	Account result = new Account();
        result.setBalance(...);
        result.setDealDate(...);
        result.setBankLocation(...);
        ...
        
        return result;
    }
}

함수를 분리하여 본인 확인 절차나 결과 데이터가 변경될 경우도 한 부분만 수정할 수 있게 되었습니다. 또한 owner.eqauls(customerName) 보다는 isOnwer()가 의미를 파악기에 더 명료합니다.


public class Dog{
	public String name;
    public int age;

	public void bark(){...}
    public void eat(){...}
    public void sleep(){...}
}
public class Cat{
	public String name;
    public int age;
    
	public void jump(){...}
    public void eat(){...}
    public void sleep(){...}
}
public class Eagle{
	public String name;
    public int age;
    
	public void fly(){...}
    public void eat(){...}
    public void sleep(){...}
}

Dog, Cat, Eagle 세 개의 클래스가 있습니다. 모든 동물들은 이름과 나이를 가지고 있고, 먹고 잠을 자기 때문에 중복 코드가 발생하고 있습니다.

public class Animal{
	public String name;
    public int age;
    
    public void eat(){...}
    public void sleep(){...}
}
public class Dog extends Animal{
	public void bark(){...}
}
public class Cat extends Animal{
	public void jump(){...}
}
public class Eagle extends Animal{
	public void fly(){...}
}

중복되는 함수와 필드들을 모두 부모 클래스로 올린 뒤, 상속을 통해 중복을 제거 하였습니다.


긴 함수, 긴 매개변수 목록
"간접 호출의 효과, 즉 코드를 이해하고, 공유하고, 선택하기 쉬워진다는 장점은 함수를 짧게 구성할 때 나오는 것이다."

책과 백기선님의 강의에서는 해당 부분이 별도의 챕터로 있지만, 공통되는 리팩터링 기법을 사용하고, 함수의 책임을 줄인다는 맥락이 같다고 생각하여 하나의 단락으로 구성하였습니다😁

예전 언어는 서브루틴을 호출하는 비용이 컸기 때문에 짧은 함수를 피했습니다. 하지만 요즘 언어들은 함수 호출 비용을 거의 없애 버렸기 때문에 짧은 함수에 대한 부담이 줄었습니다. 또한 IDE의 발전으로 함수 간의 이동 또한 쉬워졌습니다.
긴 함수를 짧은 함수로 분리한다면 함수의 코드를 전부 보지않아도 함수의 이름을 통해 문맥과 기능을 파악할 수 있다는 장점이 있습니다. 또한 하나의 함수가 하나의 역할을 수행할 수 있도록 합니다. 이러한 함수 쪼개기의 분명한 목적은 함수의 코드 길이를 줄이는 것이 아니라 함수의 의도를 더욱 명확하고 빠르게 전달하는 것 입니다.

  • 함수가 너무 많은 역할을 수행하지는 않는가?
  • 불필요한 매개 변수는 없는가?
  • 하나의 데이터 객체로 모을 수는 없는가?

긴 함수, 긴 매개변수를 발견했다면 위의 상황을 먼저 생각해봅니다. 그 다음 아래의 개선 방법들을 적용해봅니다.

  1. 함수 추출하기
    함수로 분리할 수 있는 코드는 별도의 함수로 분리합니다. 만약 주석이 있는 코드 블럭이 있다면, 주석을 함수명으로, 구현을 함수의 본문으로 옮깁니다.
  2. 매개변수 통째로 넘기기
    코드를 여러 함수들로 분리하다보면 매개변수를 많이 사용하게되어 함수 선언이 너무 길어질 수 있다. 이때는 매개변수 객체를 사용하던가 객체를 통째로 넘기는 것을 고려해봅니다.
    우리는 이미 이 방법을 자주 사용하고 있습니다. Enity 객체, DTO 객체 모두 이와 같은 맥락에 속합니다.
  3. 플래그 매개변수 제거하기
    Boolean 값을 매개변수로 받아 함수의 기능을 분기하는 부분을 두개의 함수로 분리합니다.
// before
public int discount(Customer customer, int price, boolean isVip){
	if (isVip) {
    	... //vip 고객에 대한 할인 적용
    } else {
    	... //일반 고객에 대한 할인 적용
    }
    ...
    return result;
}

// after
public int vipDiscount(Customer customer, int price){
	...
  	return result;
}
public int discount(Customer customer, int price){
	...
    return result;
}
  1. 조건문 분해하기
    if문, switch문 등 조건문 각 블럭들을 함수로 추출할 수 있는지 생각해봅니다. 조건문의 경우 코드가 길어지면 가독성이 많이 떨어지게 됩니다. 함수로 추출한다면 각 조건에 따른 코드의 흐름과 맥락을 파악하기 쉬워집니다.
  2. 반복문 쪼개기
    만약 하나의 반복문에서 여러가지 작업이 수행된다면, 각 작업을 함수로 분리한다. 하나의 반복문으로 여러 작업을 수행하는 것이 여러 반복문을 두는 것보다 효율적이라고 생각할 수 있지만, 시간복잡도 관점에서 O(N)의 반복문이 여러개로 나뉜다하여도 결국 O(N)이다. 함수를 분리하여 얻는 이점이 더 크다 생각한다.
  3. 필드 활용하기
    같은 클래스 내의 함수들이 공통적으로 매개변수로 사용하는 변수가 있다면 필드로 선언하여 사용하는 것을 고려해볼 수 있습니다.

전역 데이터

"무엇이든 많이 복용하면 독이 될 수 있다."

여기서 말하는 전역 데이터는 캡슐화 되지 않은 전역 데이터를 말합니다. 캡슐화가 되어 있지 않아 어느 곳에서나 수정이 가능할 경우, 문제가 발생했을 때 어디서 수정이 되었는지 파악이 힘들어집니다. 만약 가변적인 전역 데이터라면 캡슐화를 통해 값에 대한 수정을 제한할 수 있습니다. 불변적인 데이터라면 상수 선언을 사용합니다.

// before
public class Example{
    //캡슐화가 되어있지 않아 어느 곳에서나 접근과 수정이 가능하다.
	public static int storeCode = "29840";
}

// after (캡슐화)
public class Example{
	// 캡슐화를 통해 접근과 수정을 제한한다.
    // 될 수 있다면 setter를 통한 수정도 제한한다.
	private static int storeCode = "29840;
    
    public setStoreCode(int storeCode){
    	this.storeCode = storeCode;
    }
    public getStoreCode(int storeCode){
    	return this.storeCode;
    }
}

// after (final)
public class Example{
	// 상수 선언을 통해 불변 데이터로 사용한다.
    public static final int STORE_CODE = "29840";
}

가변 데이터

"코드의 다른 곳에서는 다른 값을 기대한다는 사실을 인식하지 못한 채 수정해버리면 프로그램이 오작동한다."

가변 데이터는 사이드 이펙트를 유발합니다. 적절한 상황이 아니라면 되도록 불변 데이터 사용을 지향하는 것이 좋다고 생각합니다. 여기서 말하는 적절한 상황이란 값을 누적 시키거나, 반복문의 인덱스로 사용되거나 하는 경우입니다. 이외의 상황에서는 가변 데이터를 줄이기 위해 고민해봐야합니다.

  1. 변수 쪼개기
    값을 누적하는 변수가 아니지만 중복되어 사용되는 변수들을 분리하여 불변 데이터로 사용할 수 있습니다.
// before
public class Example{
	private double area;
    
    public void calRectangle(double height, double width){
    	double tmp = height * width;
        area = tmp;
    }
}
// after
public class Example_2{
	private double area;
    
    public void calRectangle(double height, double width){
    	final double area = height * width;
        this.area = area;
    }
}
  1. 조회 함수와 변경 함수 분리하기
    return 값을 변경하는 로직과 해당 값을 반환하는 로직이 한 함수에 있다면 사이드 이펙트가 존재하기 때문에 두 개의 함수로 분리하는 것이 좋습니다.
// before
public class Example{
	public String checkAdultAndCharge(int age){
    	return (age > 19) ? "요금은 10000원 입니다." : null;
    }
}	
// after
public class Example{

	public String calTicketCost(int age){
    	return isAdult(age) ? "요금은 10000원 입니다." : null;
    }
	public Boolean isAdult(int age){
    	return age > 19 ? true : false;
    }
}	

가독성을 위해 아주 짧은 예제 코드를 작성하였지만, 굉장히 복잡한 성인 여부 로직과 요금 계산 로직이 있다고 가정합니다😂 만약 before 코드에서 추후 성인 여부를 판단하는 기준이 나이 뿐만 아니라 주민등록증이나 운전면허증 소지 여부가 추가된다면 매우 긴 함수가 되리라 생각합니다.
또 성인에 대한 요금이 15000원으로 인상되거나, 중간에 할인이나 세일 등 요금을 계산하는 로직이 추가된다면, 많은? 냄새를 풍기는 함수가 되었을 것 입니다.

  1. setter 함수 제거하기
    변경이 필요 없는 값에 대해서는 setter 함수를 제거하고 생성자로 변수를 받는 것을 추천합니다.
public class Person{
	private int id;
    private String name;
    
    // 변경되지 않는 값을 생성자로 받는다.
    public Person(int id){
    	this.id = id;
    }
    public getName(){
    	return name;
    }
    pubic setName(String name){
    	this.name = name;
    }
}
  1. 파생 변수를 조회 함수로 변경하기
    계산을 통해 값을 구할 수 있는 변수는 함수로 변경하여 가변 데이터를 제거할 수 있습니다. 가변 데이터에 담아서 사용하게 되는 경우 해당 변수가 어딘가에서 변경이 될 가능성이 있습니다. 이 리팩터링의 목적은 변수를 줄이는 것이 아니라 가변 데이터를 줄이는 것이기 때문에, 만약 계산 결과가 항상 똑같다면 final 변수에 담아 사용하여도 괜찮습니다.

  2. Reference Object <-> Value Object
    참조 객체를 값 객체로 변환하여 불변 데이터처럼 사용할 수 있습니다. 객체의 내부 필드를 수정하지 않고 객체를 통째로 교체하여 사용하는 방식입니다. 기존의 사용하던 필드를 final 로 선언한 뒤 setter 함수들을 모두 제거합니다. 필드 값들은 생성자를 통해 초기화합니다. 이렇게 생성된 값 객체는 필드가 수정되지 않기 때문에 어느 위치에서나 동일한 값을 보장합니다. 주의할 점은 equals()와 hachCode()를 오버라이딩해야 합니다. 필드가 모두 동일한 값을 가지는 경우 동일성을 보장하기 위함입니다.
    만약 객체의 필드 변경이 필요하고 변경 내역을 공유해야 한다면 참조 객체로 사용하는 것이 좋습니다.
    무조건적으로 모든 객체를 불변으로 변경하기 보다는 상황에 따라 적절하게 사용하여야 합니다.


뒤엉킨 변경

"소프트웨어는 자고로 소프트해야 마땅하다."

해당 냄새는 모듈화가 적절히 이루어지지 못한 상태를 의미합니다. 만약

profile
Keep Going

0개의 댓글