개요
최근 클린 코드에 대한 내용을 다루는 내 코드가 그렇게 이상한가요? 라는 책을 읽는 스터디에 참여하게 되었는데, 첫 주차 미팅에서 흥미있게 읽은 부분 일부를 정리해보려고 한다.
( + 혹시 클린 코드 책 (로버트 C. 마틴의 그거그거)에 관한 정보인 줄 알고 들어오셨다면 길 잘못 드신 거라는 삼삼한 사과의 말씀과 함께 시작하겠습니다.. 홀홀 )
클래스 응집도가 낮을 때 생기는 문제들
객체 지향에 대해 공부하다보면 자주 접하게 되는 몇 가지 권고 사항들이 있다. 그 중 하나가 바로 '결합도는 낮추고 응집도는 높여라' 다.
응집도를 높여야 하는 이유는 관련 로직들을 한 곳에 집중시켜 개발자로 하여금 코드 의미를 파악하기 쉽게 하고, 기능 변경이나 확장으로 인한 유지 보수를 쉽게 만들고, 코드 재사용을 원활하게 만들 수 있기 때문이다.
보통 공부할 때는 응집도를 높여야 하는 이유, 높이면 좋은 점 < 위주로 정리해서 봤던 것 같다. 오늘은 반대로, 데이터 클래스를 활용하여 응집도가 낮을 경우 발생하는 문제에 대해 알아보자.
🤔 데이터 클래스란?
먼저 데이터 클래스에 대해 알아보자. 아주 짧게 요약하자면, 데이터를 갖고 있기만 하는 클래스를 일컫는 말이다.
우리는 개발을 할 때 다양한 비즈니스 로직을 구현하게 되고, 그 과정에서 관련이 있는 데이터들을 클래스로 묶게 된다. 하지만 클래스에는 인스턴스 변수의 값만을 들려주고, 정작 해당 데이터를 다루는 로직은 다른 클래스에 배치하는 상황이 종종 일어난다. 예를 들어보자.
public class ContractAmount {
public int amountIncludingTax;
public BigDecimal salesRate;
public ContractAmount(int amountIncludingTax, BigDecimal salesRate) {
this.amountIncludingTax = amountIncludingTax;
this.salesRate = salesRate;
}
}
변수와 생성자를 가진 아주 간단한 클래스다. 클라이언트 코드에서 아래와 같은 코드를 작성하여 이 클래스를 사용한다고 가정한다.
public double calculateSalesPrice(int price, BigDecimal saleRate) {
if (price < 0) throw new IllegalArgumentException("가격은 양수만 가능합니다.");
if (saleRate == null) throw new IllegalArgumentException("세금 값이 필요합니다.");
ContractAmount amount = new ContractAmount(price, saleRate);
... // 생략
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
가격은 언제나 양수여야 하며, 할인율은 null이 되면 안된다는 유효성 검증 과정을 거쳐, 유효한 값을 이용해 클래스 인스턴스를 생성한다. 이후 적절한 비즈니스 로직을 거쳐 원하는 값을 리턴한다. ConstractAmount는 정말 단순히 값을 저장하는 통에 불과한 구조인 것!
와! 너무 간단한걸? 정말 아무 문제도 일어날 것 같지 않아!
안타깝지만 문제가 많이 발생한다. 데이터 클래스를 활용한 위 코드는 클래스 응집도가 낮아지는 케이스에 해당한다. 데이터 값을 다루는 로직이 클래스 밖에 있다는 점과, 동시에 데이터 클래스의 인스턴스 변수 접근 제어자가 public이므로 클래스 밖 아무데서나 해당 데이터를 변경할 수 있다는 점을 참고하여, 이 특징들이 불러올 수 있는 문제들에 대해 본격적으로 알아보자.
1️⃣ 코드 가독성 저하
위 예시는 데이터가 하나, 데이터를 사용하는 로직이 담긴 메서드가 하나라서 그렇게 가독성이 떨어진다는 느낌은 없다. 하지만 데이터 클래스를 사용하는 로직이 여러개라면 어떨까?
public double calculateSeasonOffPrice(int price) {
BigDecimal seasonOffRate = findSeasonOffPolicy();
... // 생략
if (price < 1000) throw new IllegalArgumentException("1000원 미만의 상품은 할인 대상이 아닙니다.");
ContractAmount amount = new ContractAmount(price, seasonOffRate);
... // 생략
amount = anotherSales(amount);
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
쇼핑 관련 어플은 종종 여러 할인 이벤트를 동시에 진행한다. 이를 구현하기 위해 시즌오프 세일 가격을 계산하는 메서드를 하나 더 추가했다고 가정한다.
모든 상품을 할인한다면 참 좋겠지만, 그럼 사장님은 눈물을 머금고 폐업을 준비해야 할 것이다. 최소한의 이윤을 남기기 위해 1000원 이상의 상품은 할인 대상에서 제외한다는 로직을 추가한다. (말이 안되죠? 예시입니다..)
이후 새로운 개발자 A씨가 팀에 합류하고, 온보딩 미션으로 겨울방학 세일 이벤트 구현 티켓을 받는다. 마침 기본 할인 이벤트와 시즌오프 할인 이벤트가 동시에 진행되고 있었고, 따라서 이 티켓을 진행하려면 기존 할인 이벤트에 대해 이해해야 한다. A씨는 새로운 회사에서 새마음 새뜻으로 산뜻하게 잘 해보자는 마음으로 선배들이 짜둔 코드들을 살펴보고, 곧 혼란에 빠진다.
calculateSalesPrice에선 음수값 체크만 하는데, calculateSeasonOffPrice에선 1000원 미만인지도 체크하네? 겨울방학 세일에서도 1000원 미만인지를 검사해야하나? 그런데 calculateSalesPrice의 결과값이 1000원보다 작은 경우엔 시즌오프 대상이 아닌건가? 이 price는 아무 할인도 들어가지 않은 가격을 의미하는건가? 근데 여기 두 메서드 말고 다른 메서드에서도 이 클래스를 쓰고 있는 것 같은데? 그럼 거기에 있는 유효성 검사도 고려해야 하는 건가? 어 유저가 참여할 수 있는 할인 이벤트가 각각이네?
반대로 A씨의 동기 B씨는 시즌오프 할인 로직을 수정하라는 티켓을 받는다.
시즌오프 할인율을 findSeasonOffPolicy()로 구하는구나. 근데 이 메서드가 다른 고객사의 시즌오프 할인 메서드에서도 쓰이고 있는데? 이 메서드를 고쳐도 되는건가? 어 근데 시즌 오프 할인가를 구할때 anotherSales()도 사용하네? 여기 고객사는 시즌오프 할인때 항상 이 할인 이벤트를 동시에 진행하는건가? 근데 클라이언트 코드를 보면 이미 다른 할인 이벤트도 진행 중인데?
A씨와 B씨는 혼란에 가득차 기획자에게 문의하려고 하지만, 안타깝게도 기획팀은 몰아치는 업무로 전원 9시부터 오후 6시까지 회의실에서 살 예정이라고 한다.
이렇듯 데이터와 데이터를 다루는 로직이 서로 분리되어 있고, 동시에 로직마저도 여러 곳으로 분산되어 있으면 관련 정보를 찾고 코드 의미를 파악하는데 굉장히 시간이 오래 걸린다는 가독성 저하 문제가 발생한다. 새로운 기능을 구현하려는 사람도, 기존 기능을 수정하려는 사람도 전체 처리 흐름을 파악하기 힘들어지니 협업할 때도 곤란할 것이다. 이는 시스템이 커지면 커질수록, 코드가 많아질수록 심해진다. 기능을 구현하는 것도 힘든데 그 전에 체력과 정신력을 낭비할 필요는 없겠지? 🤔
2️⃣ 코드 중복과 수정 누락
위 예시를 좀 더 이어보자. B씨는 결국 기존 코드를 수정하자니, 다른 메서드에도 영향이 갈 것 같아 기존 메서드는 그대로 두고 새로운 시즌오프 할인 메서드를 만들기로 결심한다. 작업을 마친 결과물은 아래와 같다.
// 기존 시즌오프 할인 메서드
public double calculateSeasonOffPrice(int price) {
BigDecimal seasonOffRate = findSeasonOffPolicy();
... // 생략
if (price < 1000) throw new IllegalArgumentException("1000원 미만의 상품은 할인 대상이 아닙니다.");
ContractAmount amount = new ContractAmount(price, seasonOffRate);
... // 생략
amount = anotherSales(amount);
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
// B씨가 새로 만든 시즌오프 할인 메서드
public double calculateSeasonOffPriceV2(int price) {
BigDecimal seasonOffRate = findSeasonOffPolicyV2();
... // 생략
if (price < 1000) throw new IllegalArgumentException("1000원 미만의 상품은 할인 대상이 아닙니다.");
ContractAmount amount = new ContractAmount(price, seasonOffRate);
... // 생략
amount = anotherSalesV2(amount);
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
시즌 오프 메서드를 하나 더 만들고, 여기서 추가로 호출하던 메서드들도 전부 V2버전을 만들어 이번에 부여받은 티켓의 요구사항대로 수정해두었다. 이렇게 하면, 원본 메서드를 호출하는 메서드나 원본 메서드가 호출하던 메서드에 영향을 주지 않으면서 신규 기능도 처리할 수 있지! B씨는 신나게 퇴근한다.
하지만 바로 다음날 시즌오프 대상 조건이 바뀌었다는 연락을 받는다. 이전에 시즌오프 로직을 구현한 당사자, C씨는 5000원 이상의 상품만이 시즌오프 할인 대상이며, 따라서 기존 코드를 수정해야 한다는 요청을 받고 calculateSeasonOffPrice메서드의 로직을 수정한다. 안타깝게도 1주일 동안의 장기휴가를 다녀왔다가 오늘 출근한 C씨는 B씨가 온보딩 미션으로 시즌오프 메서드를 새롭게 만들었다는 사실을 모른다. 따라서 B씨의 V2메서드의 존재를 모른채, 자신이 만들었던 메서드만 쏙 수정한다.
// 기존 시즌오프 할인 메서드
public double calculateSeasonOffPrice(int price) {
BigDecimal seasonOffRate = findSeasonOffPolicy();
... // 생략
if (price < 5000) throw new IllegalArgumentException("5000원 미만의 상품은 할인 대상이 아닙니다.");
ContractAmount amount = new ContractAmount(price, seasonOffRate);
... // 생략
amount = anotherSales(amount);
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
// B씨가 새로 만든 시즌오프 할인 메서드
public double calculateSeasonOffPriceV2(int price) {
BigDecimal seasonOffRate = findSeasonOffPolicyV2();
... // 생략
if (price < 1000) throw new IllegalArgumentException("1000원 미만의 상품은 할인 대상이 아닙니다.");
ContractAmount amount = new ContractAmount(price, seasonOffRate);
... // 생략
amount = anotherSalesV2(amount);
return amount.amountIncludingTax * amount.salesRate.doubleValue();
}
얼마 후 C씨와 B씨의 코드 모두가 배포되었고, 쇼핑몰에선 5000원 미만의 일부 상품도 시즌 오프 할인이 들어가는 버그를 마주한다. 큰맘먹고 할인 이벤트를 계획했던 사장님은 잘못된 할인 로직으로 인해 10억 적자를 마주하고 곧 파산했으며, 화가 나 개발사를 고소하고, B씨와 C씨는 해고 당한다.
이 예시는 꽤 가볍고 우습지만, 실제로 소프트웨어 규모가 클 수록, 담당 부서가 분산 될 수록, 관련 코드가 멀리 떨어져 있을 수록 개발자가 코드를 파악하기 어렵다는 것은 확실하다. 1에서 언급한 가독성 저하 문제 때문이다.
이렇게 전체 코드를 파악하기 힘들어진 개발자는 이미 기능이 구현되어 있는데도 '어? 왜 이 기능이 없지?' 하고 같은 로직을 새롭게 구현할 수 있다. 불쌍한 B씨처럼 기존 코드를 수정하지 않고 새롭게 만드는 경우도 있고. 이와 같이 동일한 기능을 여러 곳에 구현하는 것을 코드 중복이라고 부른다.
코드 중복은 개발자의 생산성을 저하시키고 가독성 저하를 가속시킨다는 점에서 이미 심각한 친구지만, 또 새로운 문제를 야기한다. 바로 C씨가 겪은 수정 누락 문제다. 같은 기능이 여러 곳에 구현되어 있고, 클라이언트는 각각의 기능들을 호출하며 특정 기능을 동작시킨다. 이 과정에서 비즈니스 로직이 변경되면 시스템 곳곳에 있는 관련 기능들을 모두 수정해야 하는데, C씨와 같이 미처 파악하지 못한 코드는 변경하지 못하는 수정 누락 문제가 발생하는 것이다.
3️⃣ 쓰레기 객체 : 초기화되지 않은 상태
응집도가 낮은 코드가 이미 많이 뚜까 얻어 맞은 것 같지만 아직 안끝났다.
ContractAmount amount = new ConstractAmount();
System.out.println(amount.saledRate.toString());
아주 간단한 코드다. 이 코드의 실행 결과는 뭘까? 바로 NPE다 😮
BigDecimal 값은 따로 초기화가 되지 않으면 null값이 할당된다. 따라서 null에 대해 toString() 요청이 들어왔으니 NullPointerException이 발생하는 것!
위 예시야 아무 값 없이 만들어놓고 toString()을 던지는 경우가 뭐가 있냐, 억까다 라고 생각할 수 있다. 하지만 ContractAmount을 매개변수로 받거나, 계산 로직 등을 타는 경우라면 어떨까? ContractAmount가 input 값 없이 디폴트 값을 갖는 상태로 생성되는 경우는?
public ContractAmount() {
this.amountIncludingTax = 0;
this.salesRate = null;
}
추가로 초기화되어야 하는 클래스라는 것을 모르면 금방 클래스가 불완전해지고, 코드에는 버그가 나기 쉬워진다. 이와 같이 '초기화하지 않은 상태가 발생할 수 있는 클래스' 또는 '초기화하지 않으면 쓸모 없는 클래스'를 안티 패턴 '쓰레기 객체'라고 부른다. 쓰레기 객체 또한, 클래스의 값을 할당하는 과정을 외부에 위임하면서 응집도가 떨어져 발생한 문제다.
4️⃣ 잘못된 값 할당
아직 한 방 남았다 🔫
ContractAmount 클래스의 amountIncludingTax의 자료형은 int지만, 의미상으론 세금을 포함한 가격이기 때문에 실제론 음수가 할당되면 안된다. 하지만 사용하는 입장에선 int 자료형이므로 얼마든지 음수 값을 할당할 수 있는 상황이다. 실제로 아래 코드는 컴파일 에러 없이 아주 잘 동작한다.
ContractAmount amount = new ConstractAmount(-1000, null);
ContractAmount를 사용하는 코드에서 음수 여부를 체크하는 유효성 검증 로직을 추가할 수도 있지만, 그렇게 하려면 이 클래스를 사용하는 개발자 모두가 음수를 체크해야 한다는 점을 알고 있어야 한다. 동시에 여러 메서드에 이러한 유효성 검증 코드가 반복적으로 사용되는 코드 중복 문제가 발생할 수도 있다.
정리하자면 요구사항에 맞지 않는 값이 클래스에 할당될 수 있으며, 이를 해결하려다 코드 중복, 수정 누락, 가독성 저하를 줄줄이 만날 수 있는 상황이 된다 🫠
5️⃣ 정리
정리하자면, 클래스 응집도가 떨어지면 동일한 값을 다루는 로직이 사방 팔방에 구현되어 전체 흐름을 파악하기 힘든 가독성 저하 문제가 발생하고, 이로 인해 같은 기능을 여러번 구현하는 코드 중복이 발생할 수 있으며, 이는 곧 요구사항 변경이 생겼을 때 수정 누락을 야기한다. 데이터 값에 대한 요구사항 처리를 클래스 밖에서 처리하므로 값이 초기화되지 않아 비정상적인 결과를 마주할 수 있고, 동시에 요구사항에 맞지 않은 값이 할당되어 논리적 오류가 발생할 수 있다.
이렇듯 응집도가 떨어지면 전체 기능 응집도가 떨어져 개발자의 생산성이 낮아지고, 코드 유지보수가 어려워진다는 점을 이해하면 자연히 응집도는 높을수록 좋다는 것을 알 수 있다 💪
응집도가 높아야 한다는 건 알겠어! 그럼 응집도를 어떻게 높일 수 있을까? 응집도를 높일 수 있는 방법은 여러가지가 있지만, 이번 포스팅에선 그 중 두가지만 소개해보려고 한다. 바로 완전 생성자와 값 객체 방법이다.
완전 생성자 (Complete Constructor)
사실 책에선 완전 생성자와 값 객체가 디자인 패턴의 일종이라고 소개했는데, 딱히 다른 곳에선 그런 언급이 없더라..? 완전 생성자라는 워딩 자체를 찾아보기가 어려웠다. (영어로 검색해도!)
일단 책에선 완전 생성자란 잘못된 상태로부터 클래스를 보호하기 위한 디자인 패턴이라고 소개했지만, 이 워딩이 많은 사람들이 합의하고 사용하는 사전화된 개념인지, 디자인 패턴으로 분류가 된 것인지 확실하지 않으므로 생성자 내부에 유효성 검사 로직을 추가하는 등의 방법을 이용해 잘못된 상태로부터 클래스를 보호하는 방법이라고 이해하면 적당할 것 같다.
ConstractAmount 클래스에 완전 생성자 패턴을 적용해보자.
public class ContractAmount {
final int amountIncludingTax;
final BigDecimal salesRate;
public ContractAmount(final int amountIncludingTax, final BigDecimal salesRate) {
if (amountIncludingTax < 0) throws new IllegalArgumentException("음수를 할당할 수 없다.");
if (salesRate == null) throws new IllegalArgumentException("할인율이 필요하다.");
this.amountIncludingTax = amountIncludingTax;
this.salesRate = salesRate;
}
}
변경된 ConstractAmount 클래스를 뜯어보자.
먼저 인스턴스 변수를 모두 초기화했을 때만 객체를 생성할 수 있게 매개변수를 가진 생성자를 만들고, 생성자 내부에 각 변수가 유효한지를 체크하는 가드를 추가했다. 이를 통해 모든 ConstractAmount 객체는 항상 정상 값을 가진 완전한 상태로만 생성된다. 앞으로 개발자는 ContractAmount의 값을 검증하기 위해 시스템 전체를 뒤질 필요도, 기획자를 찾을 필요도 없다. 그냥 생성자만 보면 된다.
뿐만 아니라, 인스턴스 변수 각각에 final을 붙여 불변 상태로 만들었다. 만약 값을 재할당하려면 생성자를 통해 새로운 객체를 만들어야 한다. 이를 통해 맨 처음 완전한 상태로 초기화된 객체에 이후 휴먼 에러 등으로 잘못된 값이 재할당 되는 것을 막을 수 있다.
이렇게 완전 생성자를 통해, ContractAmount는 항상 정상 값을 가진 상태로만 존재하며, 생성 이후에도 잘못된 상태를 방지할 수 있게 되었다. 이는 생성과 관련된 로직 - 유효성 검증 - 을 클래스 밖이 아닌 클래스 안에 위치하여 응집도를 높였기에 가능한 일이라는 것도 기억해두자.
값 객체 (Value Object)
ContractAmount에서 amountIncludingTax의 값을 더하는 상황이 있다고 가정하자. 뭐 장바구니 같은 곳에 여러 객체가 추가되어 총 합계를 계산해야 하는 상황이겠지? 클래스 응집도를 높이기 위해 증가 로직을 클래스 내부 메서드에 구현했다.
public class ContractAmount {
final int amountIncludingTax;
final BigDecimal salesRate;
public ContractAmount(final int amountIncludingTax, final BigDecimal salesRate) {
if (amountIncludingTax < 0) throws new IllegalArgumentException("음수를 할당할 수 없다.");
if (salesRate == null) throws new IllegalArgumentException("할인율이 필요하다.");
this.amountIncludingTax = amountIncludingTax;
this.salesRate = salesRate;
}
ContractAmount add(final int other) {
final int added = amountIncludingTax + other;
return new ContractAmount(added, salesRate);
}
}
이렇게 하면 모든 증가 로직이 ContractAmount의 add()메서드를 이용할테니 나중에 관리하기도 편하겠지? 물론 좋은 시도지만, 100점짜리 답은 아니다. 이 메서드를 호출하는 클라이언트 코드를 생각해보자.
ContractAmount amount = new ContractAmount(3000, salesRate);
int newPrice = -1000;
amount.add(newPrice);
현재 요구사항은 물건들의 값을 더하는 것이고, 물건에는 음수 값이 할당될 수 있으니 위 코드는 논리적으로 잘못된 코드이다. 하지만 int 자료형이 음수를 갖는 것은 전혀 잘못된 일이 아니기 때문에 정상적으로 컴파일된다.
이렇게 의미가 다른 여러 값들에 int나 String과 같은 기본 언어 자료형을 사용하면 쉽게 헷갈릴 수 있다. 동시에 디버깅하기도 힘들다. 분명 어디서 이상한 값이 할당되었는데 대체 어디서 들어간건지, 브레이킹 포인트를 하나하나 찍어서 관측하는 수밖에 없기 때문이다.
이는 결국 int 자료형이 ContractAmount의 요구사항을 충족할 수 없기 때문에 생긴 문제다. 따라서 우리는 독자적인 자료형을 이용해, 의미가 다른 값의 전달 자체를 컴파일 단계에서 막으면 어떨까? 라는 생각을 하게 된다. 이것이 바로, 값을 클래스(자료형)으로 나타내는 패턴인 값 객체이다. 위 로직을 값 객체를 이용하여 리팩토링해보자.
public class ContractAmount {
final int amountIncludingTax;
final BigDecimal salesRate;
public ContractAmount(final int amountIncludingTax, final BigDecimal salesRate) {
if (amountIncludingTax < 0) throws new IllegalArgumentException("음수를 할당할 수 없다.");
if (salesRate == null) throws new IllegalArgumentException("할인율이 필요하다.");
this.amountIncludingTax = amountIncludingTax;
this.salesRate = salesRate;
}
ContractAmount add(final ContractAmount other) {
final int added = amountIncludingTax + other.amountIncludingTax;
return new ContractAmount(added, salesRate);
}
}
이렇게 하면, add메소드에 넘겨지는 값은 항상 ContractAmount의 생성자를 통해 만들어진 값일 것이다. 그리고 생성자를 통해 만들었으므로 완전생성자를 통해 얻는 이점 - 항상 완전한 상태의 값이므로 add 메서드를 사용하는 클라이언트는 이제 잘못된 논리 에러가 발생할 가능성에 대해 걱정을 내려놓아도 된다.
정리하자면 값 객체는 값과 로직을 응집도 높은 구조로 만들어, 의미가 다른 값이 섞이는 것을 방지할 수 있는 방법인 것!
레퍼런스
'Develop > Etc' 카테고리의 다른 글
[Cache/Spring] EhCache vs Cache2k vs Caffeine 캐시 비교하기 (0) | 2024.08.12 |
---|---|
[객체지향] 일급 컬렉션 (First Class Collection) (0) | 2023.11.13 |
[Design Pattern] Facade 패턴과 이모저모 (2) | 2023.07.12 |