상당히 길고 복잡한 기능을 수행하는 로직을 만들었다. 요구사항이 변경되면서 엔티티 설계가 함께 변경되었는데 이때 만든 코드가 스파게티처럼 얽혀 있어 수정이 어려웠다. 이때의 리팩토링 경험을 공유해본다.




접근제한자 public, private으로 추상화레벨을 구분하기

이팩티브자바, 클린코드 등에서 메소드의 추상화 레벨을 통일해야한다고 말한다. 추상화레벨은 기본적으로 2가지(public, private)로 구분하는 편이 가독성이 좋아보인다.

class CoffeeMachine {

    //추상화 수준 높음
    public void makeCoffee() {
        brewCoffee();
        pourCoffee();
    }

    //추상화 수준 낮음
    private void brewCoffee() {
    }

    private void pourCoffee() {
    }
}




추가로 추상화 레벨을 구분하려면 클래스를 분리하기

모든 코드가 단 2개의 추상화 레벨만 가진다면 좋겠지만 이미 한번 추상화 레벨을 분리한 private 메소드에서 다시 저수준의 메소드를 분리할 일이 생긴다. 클래스 내부에 여러 추상화 레벨이 존재하면 가독성이 떨어지고 접근제한자로 추상화 레벨을 구분한 의미가 없어진다. 이럴 때는 낮은 레벨의 로직은 하위 클래스로 분리하는 편이 좋다고 생각한다.

class CoffeeMaker { // before

    //추상화 수준 높음
    public void makeCoffee() {
        brewCoffee();
        pourCoffee();
    }

    //추상화 수준 중간 // private 메소드 내에서 수준이 혼재
    private void brewCoffee() {
        getHotWaterPot();
        dripCoffee();
    }

    private void pourCoffee() {
        getMugCup();
        transferCoffeeToCup();
    }
    
    //추상화 수준 낮음 // private 메소드 내에서 수준이 혼재
    private getHotWaterPot(){}
    private dripCoffee(){}
    private getMugCup(){}
    private transferCoffeeToCup(){}
}

class CoffeeMaker {//after
    private BrewingSupporter brewingSupporter;
    private PouringSupporter pouringSupporter;
    
    public void makeCoffee() {
        brewingSupporter.brewCoffee();
        pouringSupporter.pourCoffee();
    }
    
}

class BrewingSupporter{ // 클래스분리
    
    public void brewCoffee() {
        getHotWaterPot();
        dripCoffee();
    }
    
    private getHotWaterPot(){}
    private dripCoffee(){}

}

class PouringSupporter{ // 클래스분리
    
    public void pourCoffee() {
        getMugCup();
        transferCoffeeToCup();
    }

    private getMugCup(){}
    private transferCoffeeToCup(){}
}




접근제한자로 추상화레벨을 구분한다는 것의 생각

사실 접근제한자는 캡슐화를 위한 것이다. 캡슐화는 노출되어야할 메소드와 그렇지 않을 메소드를 분리하는 것이다. 접근제한자는 추상화레벨을 구분할 목적으로 만든 것이 아니지만 결국 객체 외부에가 호출되어야 하는 보통 추상화 레벨이 높은 메소드로써 일종의 창구역할을 하기 때문에 통용적으로 그렇게 사용하는 것 같다.




클래스 간의 추상화레벨은 패키지로 정리하기

앞서 얘기한 것처럼 한 클래스 내부의 3개 이상의 추상화 레벨이 존재할 경우 클래스를 분리하는 것이 좋다고 얘기했다. 그러나 같은 패키지 경로에 여러 클래스가 생기게 되고 클래스간의 추상화 레벨이 다르지만 같은 패키지에 혼재되어 있는 문제가 생긴다.


package cafe.coffee;

public class CoffeeMaker {
    private BrewingSupporter brewingSupporter;
    private PouringSupporter pouringSupporter;
    
}

package cafe.coffee;

public class BrewingSupporter{  //외부노출
}

package cafe.coffee;

public class PouringSupporter{// 외부 노출
}

만약 다른 패키지인 cafe.Barista 에서 cafe.coffee 를 바라볼 때 3개의 클래스 모두 개방되어 있다면 커피를 만들기 위해서 추상화가 낮은 BrewingSupporter 나 PouringSupporter가 직접 사용되는 문제가 발생한다. 따라서 클래스의 접근제한자도 제한해두는 것이 좋다. 또는 메소드의 접근제한자도 조작가능하다.


package cafe.coffee;

public class CoffeeMaker {
    private BrewingSupporter brewingSupporter;
    private PouringSupporter pouringSupporter;
    
}

package cafe.coffee;

default class BrewingSupporter{  //외부노출 안됨
}

package cafe.coffee;

default class PouringSupporter{// 외부 노출 안됨
}




꼭 빈으로 등록할 필요는 없다.

추상화레벨이 높고 외부로 노출되는 퍼블릭 클래스이며 여러 의존성 주입에 참여한다면 스프링에서 빈으로 등록될 후보가 된다. 그러나 챗지피티가 설명해주는 것처럼 리팩토링을 통해 분리한 클래스들을 빈으로 등록해야할지는 고민해야한다.

// from GPT-4o
. 빈으로 등록할 필요가 있는 경우:
의존성 주입이 필요한 경우: 다른 클래스에 의존하는 서비스나 컴포넌트들은 스프링이 제공하는 의존성 주입(DI)을 활용할 수 있도록 빈으로 등록하는 것이 좋습니다. 이는 테스트 용이성과 유지보수성을 높여줍니다.
트랜잭션 관리가 필요한 경우: 데이터베이스 연동 로직이나 트랜잭션 관리가 필요한 경우, 스프링의 @Transactional 같은 어노테이션을 활용할 수 있는 빈 등록이 적합합니다.
AOP 적용이 필요한 경우: 로깅, 보안, 트랜잭션과 같은 횡단 관심사(Aspect-Oriented Programming, AOP)를 적용하려면 대상 클래스가 스프링 빈으로 등록되어야 합니다.
상태가 없는 서비스 로직: 주로 비즈니스 로직을 처리하는 서비스 계층(예: @Service)은 상태가 없는 경우가 많으며, 이러한 로직은 스프링 빈으로 관리하여 재사용성과 테스트 용이성을 높일 수 있습니다.
스프링 컨테이너에서 관리되어야 할 필요가 있을 때: 다른 스프링 기능을 활용해야 하거나 생명주기, 프로파일 등에 따른 설정이 필요하다면 빈으로 등록하는 것이 적합합니다.
2. 빈으로 등록할 필요가 없는 경우:
단순한 헬퍼 유틸리티 클래스: 계산, 포맷팅, 변환 등 간단한 헬퍼 메서드를 포함한 클래스들은 굳이 스프링 빈으로 등록할 필요가 없습니다. 스태틱 메서드로 구현하거나, 로직이 명확한 경우 직접 호출하여 사용할 수 있습니다.
상태를 가지는 객체: 스프링 빈은 주로 상태가 없는(stateless) 로직에 적합합니다. 상태를 가지는 객체(예: 사용자 세션 관리 등)는 빈으로 등록하는 것이 적절하지 않을 수 있습니다. 이런 경우는 별도의 인스턴스를 생성하여 사용하거나 상태 관리 전략을 따로 적용하는 것이 더 좋습니다.
성능 이슈가 발생할 수 있는 경우: 일부 로직은 빈으로 등록하지 않고 필요한 곳에서 직접 생성하는 것이 성능에 유리할 수 있습니다. 예를 들어, 초기화가 복잡하고 자주 사용되지 않는 객체는 스프링이 관리하지 않고 필요할 때마다 생성하는 방식이 적합할 수 있습니다.
3. 추가 고려사항:
스프링 빈 수 증가에 따른 오버헤드: 너무 많은 클래스를 빈으로 등록하면 애플리케이션 시작 시 스프링 컨텍스트 초기화 시간이 늘어나거나, 메모리 사용량이 증가할 수 있습니다.
로직의 독립성: 빈으로 관리되지 않는 클래스는 스프링 컨텍스트와 독립적으로 동작할 수 있으므로, 애플리케이션 내부적으로 독립적이어야 하는 컴포넌트들은 빈으로 등록하지 않는 것이 유리할 수 있습니다.

따라서 모든 클래스를 빈으로 등록하는 것보다. 상태를 저장하지 않는 유틸리티 클래스이거나 외부 의존성 주입이 없는 경우는 그대로 두고 메소드를 static으로 두는것이 현명하다.


package cafe.coffee;

@Component
public class CoffeeMaker {
    //필드 참조가 사라짐. 
    //private BrewingSupporter brewingSupporter;
    //private PouringSupporter pouringSupporter;

    public void makeCoffee() {
        BrewingSupporter.brewCoffee();
        PouringSupporter.pourCoffee();
    }
    
}

default class BrewingSupporter{ 

    protected static void brewCoffee() {
        getHotWaterPot();
        dripCoffee();
    }

    private static getHotWaterPot(){}
    private static dripCoffee(){}

}

default class PouringSupporter{ 

    protected static pourCoffee() {
        getMugCup();
        transferCoffeeToCup();
    }

    private static getMugCup(){}
    private static transferCoffeeToCup(){}
}




책임분리, 추상화 레벨에 맞는 파라미터와 메서드 시그니처

만약 게시글의 본문 HTML 을 process할 기능을 도입한다고 하자. html 문자열을 받아서 내부의 다양한 종류 태그를 삭제하는 로직을 구현하는 스프링 빈 컴포넌트를 설계한다면 다음과 같은 모습으로 나올 수 있다.


@Component
public class ContentProcessor{
    
    public String process(String html){
        validateHtmlCompatibility(html);
        return deleteImgTagFrom(html);
    }
    
    private validateHtmlCompatibility(String html) throws IllegalArgumentException{
        //
    }
    
    private String deleteImgTagFrom(String html){
        //
    }
    
}

보기엔 별 문제가 없어보이지만 나는 다음과 같은 방식이 더 올바르다고 생각한다.


@Component
public class ContentProcessor {

    public HtmlContent process(HtmlContent html) {
        return html.deleteTag("img");
    }

}


public class HtmlContent {
    private String source;

    public HtmlContent(String source) {
        if (!isCompatible(source)) {
            throw new IllegalArgumentException();
        }

        this.source = source;
    }


    private boolean isCompatible(String source) {
        //html 소스를 검증함
    }

    public void deleteTag(String tagName) {
        //source 필드의 태그를 찾고 태그를 제거한 후 업데이트함
    }
}
  • 파라미터로 받는 String은 HtmlContent라는 좀더 높은 추상화 레벨의 DTO로 변경했다. 따라서 ContentProcessor 를 사용하는 비지니스 로직에서는 좀 더 추상화 레벨이 높은 객체를 다룰 수 있다.
  • String을 래핑한 dto인 HtmlContent 생성자에서 문자열 소스의 검증을 한다. 따라서 ContentProcessor 검증을 할 책임을 제거할 수 있다.
  • ContentProcessor 는 process()는 HtmlContent Dto를 파라미터로 갖는다. String 타입으로 인자를 받는 것보다 가독성이 좋다. 또한 부분별한 문자열이 들어가는 것을 막는다.
  • ContentProcess는 HtmlContent의 메소드를 사용한다. 만약 Process할때 다른 태그를 삭제하거나 넣거나 하는 작업이 생긴다면 HtmlContent에 추가 메소드를 생성하고 HtmlContent에서는 그 메소드를 사용해서 Process()를 구현하면 된다.

물론 ContentProcess의 process()를 그냥 HtmlContent에 두는 방법도 있다. 하지만 이 방법은 좋지 못하다. 우선 만약 컨텐츠를 프로세싱 하는 것이 비니지스 로직과 관련되어 있다면 Dto가 비지니스 로직을 갖게 된다. 이는 DTO의 변경이 비니지스 로직의 변경을 초래한다. 이 경우 DTO는 엔티티가 되므로 신중해야한다. 또한 해당 로직이 다른 객체와의 협력이 필요한 경우 DTO가 다른 외부 클래스에 대한 의존성을 갖게된다. 이러한 의존성은 최소화하는게 중요하다.

DTO에 넣어두는 메소드는 요구사항에 따라서 변경가능성이 낮고 외부 클래스의 의존성이 낮은 메소드여야 한다고 생각한다.




리팩토링에 따라 증가되는 레이어

지속 성장 가능한 소프트웨어를 만들어가는 방법

나는 김재민님의 글을 보면서 레이어드 아키텍처에 대해서 많은 고민을 하게 되었다. 앞서 진행한 리팩토링들은 모두 클래스 수를 증가시킨다. 이렇게 하나씩 낮은 레벨의 클래스를 증가시키면 이를 수용할 레이어는 어디로 해야할까?

늘상하게 되는 고민이지만 정답은 없는 것 같다. 다만 분리한 낮은 레벨의 구현 레벨의 클래스가 다른 곳에 사용되지 않는다면 그것을 사용하는 클래스에 가까이 두는 것이 좋다고 생각한다. 앞서 커피머신의 기능 중 Brew 와 Pour 기능을 분리한 클래스의 경우 접근제한자로 외부노출을 차단시키고 같은 레이어와 패키지 안에 두었다.

그러나 외부에서, 다른 도메인에서 그 기능을 불러오게 된다면 로직의 변경가능성을 고려해서 이것을 누구나 접근가능한 util 클래스로 두는 방법도 있다. 이런 경우 패키지는 달라지며 접근제한자도 개방된다. 이렇게 된다면 아마도 Implement 레이어에 그대로 위치시키되 그 아래에 utility 레이어를 추가로 두는 방향이 있을 것이다.




핵심은 읽기 좋고 변경에 강한 코드

지금까지 추상화 레벨 관점에서의 리팩토링을 해보았다. 어디까지난 읽기 좋고 변경에 유연한 코드를 만드는 것이 핵심이다.