045. (Pragmatic Unit Testing in Kotlin with JUnit) 9. 리팩토링 - 단일 책임 원칙, 명령 질의 분리 원칙

9. 리팩토링 - 단일 책임 원칙, 명령 질의 분리 원칙

원제 : Bigger Design Issues

이전 포스팅에서는 특정 메서드를 추출하는 방법에 대서 알아보았다.

이처럼 리팩토링을 통해 코드의 유지 보수 비용을 지속적으로 낮추는 것은 매우 중요하다.

이번에는 또 다른 방법인 하나의 클래스에 하나의 책임만을 부여하는 단일 책임 원칙과 명령 질의 분리 원칙에 대해서 알아보자.

9.1. Profile 클래스와 SRP

원제 : The Profile Class and the SRP

지금 까지 작성한 Profile 클래스를 살펴보자.

class Profile(val name: String) {
    private val answers = mutableMapOf<String, Answer>()
    private var score = 0

    fun add(answer: Answer) {
        answers[answer.questionText] = answer
    }

    fun matches(criteria: Criteria): Boolean {
        calculateScore(criteria)
        return if (doesNotMeetAnyMustMatchCriterion(criteria)) false else anyMatches(criteria)
    }

    private fun doesNotMeetAnyMustMatchCriterion(criteria: Criteria): Boolean {
        for (criterion in criteria) {
            val match = criterion.matches(answerMatching(criterion))
            if (!match && criterion.weight === Weight.MustMatch) {
                return true
            }
        }
        return false
    }

    private fun calculateScore(criteria: Criteria) {
        score = 0
        for (criterion in criteria)
            if (criterion.matches(answerMatching(criterion)))
                score += criterion.weight.value
    }

    private fun anyMatches(criteria: Criteria): Boolean {
        var anyMatches = false
        for (criterion in criteria) anyMatches = anyMatches or criterion.matches(answerMatching(criterion))
        return anyMatches
    }

    private fun answerMatching(criterion: Criterion): Answer {
        return answers[criterion.answer.questionText]
    }

    fun score(): Int {
        return score
    }

    fun classicFind(pred: Predicate<Answer?>): List<Answer> {
        val results = mutableListOf<Answer>()
        for (answer in answers.values) {
            if (pred.test(answer)) {
                results.add(answer)
            }
        }
        return results
    }

    override fun toString(): String {
        return name
    }

    fun find(pred: Predicate<Answer?>?): List<Answer> {
        return answers.values.stream()
            .filter(pred)
            .collect(Collectors.toList())
    }
}

이 클래스의 문제는 무엇일까?

Profile 클래스의 역할을 생각해보면, 회사 혹은 어떤 사람에 대한 정보를 추적하고 관리하고 있음을 알 수 있다.

이러한 정보들은 시간이 지나면서 추가되거나 제거되는 등의 변경이 발생할 수 있다.

또한 정보의 집합이 주어진 프로파일과 매칭되는지를 검증해보고 매칭 점수를 계산해 알려주고 있다.

즉 Profile 클래스는 위의 두 가지 책임을 가지고 있으며, 이는 객체지향 설계 측면에서의 단일 책임 원칙(Single Responsibility Principle) 을 위반하고 있음을 말한다.

참고 객체지향과 단위 테스트 측면에서 단일 책임 원칙에 대해서는 아래 포스팅에서도 다루었다.

• (The Essence of Object-Orientation) 1. 협력과 역할 그리고 책임
• (Unit Test Principles) 7. 가치있는 단위 테스트를 위한 리팩토링

그럼 이제 리팩토링을 통해 Profile 클래스의 책임을 하나로 만들어보자.

9.2. 클래스 추출 기법

원제 : Extracting a New Class

Profile 클래스의 책임을 다시 정리해보자.

1. 프로파일에 관한 정보 추적
2. 조건 집합이 프로파일에 매칭되는 지와 그 점수의 판단

이 책임들을 각각의 클래스로 분리하여 단일 책임 원칙을 준수해보자.

먼저 매칭 점수를 계산하는 calculateScore() 로직을 별도로 분리해보자.

분리할 클래스는 MatchSet 으로 명명한다.

이후 Profile 클래스에서의 MatchSet으로의 접근은 아래와 같이 matches() 클래스를 통해 객체를 생성하여 접근하도록 한다.

fun matches(criteria: Criteria): Boolean {
  score = MatchSet(answers, criteria).score()
  
  return if (doesNotMeetAnyMustMatchCriterion(criteria)) {
    false
  } else {
    anyMatches(criteria)
  }
}

이제 MatchSet 클래스를 작성할 차례다.

class MatchSet(
    private val answers: Map<String, Answer>,
    criteria: Criteria,
) {
    private var score = 0

    init {
        calculateScore(criteria)
    }

    fun score() = score

    private fun calculateScore(criteria: Criteria) {
        for (criterion in criteria) {
            if (criterion.matches(answerMatching(criterion)))  {
                score += criterion.weight.value
            }
        }
    }

    private fun answerMatching(criterion: Criterion): Answer {
        return answers[criterion.answer.questionText]
    }
}

이제 매칭을 점수화하는 모든 로직은 MatchSet 클래스의 책임으로 주어지게 되었다.

매칭 여부를 검증하는 코드도 위임해보도록 하자.

class MatchSet(
    private val answers: Map<String, Answer>,
    private val criteria: Criteria,
) {
    private var score = 0

    init {
        calculateScore(criteria)
    }

    fun score() = score

    private fun calculateScore() {
        for (criterion in criteria) {
            if (criterion.matches(answerMatching(criterion)))  {
                score += criterion.weight.value
            }
        }
    }

    private fun answerMatching(criterion: Criterion): Answer {
        return answers[criterion.answer.questionText]
    }


    fun matches(): Boolean {
        return if (doesNotMeetAnyMustMatchCriterion()) {
            false
        } else {
            anyMatches()
        }
    }

    private fun doesNotMeetAnyMustMatchCriterion(): Boolean {
        // ...
        for (criterion in criteria) {
            val match = criterion.matches(answerMatching(criterion))
            if (!match && criterion.weight === Weight.MustMatch) return true
        }
        return false
    }

    private fun anyMatches(): Boolean {
        // ...
        var anyMatches = false
        for (criterion in criteria) {
            anyMatches = anyMatches or criterion.matches(answerMatching(criterion))
        }
        return anyMatches
    }
}

이제 MatchSet 클래스는 매칭 요청을 처리하는 요구사항을 모두 만족하며, Criteria 객체도 프로퍼티로 가지고 있기에 많은 매개변수를 제거할 수 있게 되었다.

이처럼 하나의 클래스가 하나의 책임만을 가지게 되면, 동작의 이해는 물론 단순하고 간결한 코드로 객체를 표현할 수 있게 된다.

9.3. 명령-질의 분리 원칙

원제 : Command-Query Separation

리팩토링 후 Profile 클래스의 matches() 메서드는 아래와 같다.

fun matches(criteria: Criteria): Boolean {
  val matchSet = MatchSet(answers, criteria)
  score = matchSet.score()
  return matchSet.matches()
}

위 메서드에는 계산된 점수를 Profile 객체의 저장하는 부작용이 존재한다.

이 부작용을 없애려면, 점수를 계산하는 것과 매칭 여부를 검증하는 것을 분리해야한다.

이처럼 어떤 값을 반환하면서 부작용을 발생시키는 메서드는 명령-질의 분리 원칙(command-query separation) 을 위반한다.

명령-질의 분리 원칙은 메서드는 명령만을 실행하거나, 어떠한 질의에 대한 반환하거나 둘 중 하나의 동작만을 수행해야함을 말한다.

이를 해소하기 위해 아래와 같이 리팩토링할 수 있다.

class Profile(
    private val name: String
) {
    private val answers = mutableMapOf<String, Answer>()

    fun name() = name

    fun add(answer: Answer) {
        answers[answer.questionText] = answer
    }
    
    fun getMatchSet(criteria: Criteria): MatchSet = MatchSet(answers, criteria)

    override fun toString(): String {
        return name
    }

    fun find(pred: Predicate<Answer?>?): List<Answer> {
        return answers.values.stream()
            .filter(pred)
            .collect(Collectors.toList())
    }
}

이제 Criteria 객체를 넘길대 새로운 MatchSet 객체를 생성하고 반환하는 메서드를 추가하여 명령과 질의를 분리할 수 있게 되었다.