🔧 목표 금액 유즈 케이스 리팩토링

[psychology50]

작업 이유

💡 PR 제목에 "Api: "를 붙이지 않은 이유는 중점 확인 사항에 적어두겠습니다.

• as-is. 테스트 실행 시간이 너무 오래걸림. 길면 3분이 넘는 경우도 존재함.
• why? 통합 테스트를 불필요하게 많이 작성함.
• why? 서비스 계층 단위 테스트를 작성하는 게 너무 힘듦.
• why? 의존성 주입이 너무 복잡하고, 테스트 케이스 스크립트 라인 수가 너무 길어짐
• why? 아키텍처가 잘못되었기 때문이라고 판단
• how? 파사드 패턴을 보다 철저하게 준수하여, 서로 다른 UseCase에 변화에 의한 영향을 줄이도록 한다.

작업 사항

1️⃣ 문제의 원인

(밑에 시퍼런 점선으로 둘러쌓인 것들은 테스트 해야 할 대상을 의미. 각각 하나의 클래스 혹은 파일이라 생각해도 무방)

• 디스코드와 블로그에서 말씀드린 것처럼 비니지스 로직을 UseCase + Service 클래스가 함께 수행합니다.
• 결국 단위 테스트 타겟이 UseCase가 되는데, 의존성으로 인한 문제가 발생합니다.
  • ex. 목표 금액 삭제 테스트에는 사용자 서비스 빈이 필요없음에도, 함께 주입 받아야 함.
  • 만약 다른 빈이 주입되면, 기존의 모든 테스트 빈 주입 에러로 인해 실패함.

2️⃣ 개선 방법

• UseCase는 더 이상 비지니스 로직에 관여하지 않으며, 파사드 패턴의 라우터 역할만을 충실히 수행합니다.
• 이제 단위 테스트 타켓이 UseCase가 아닌 Service가 되므로, 다른 빈이 UseCase에 추가된다고 해도 다른 테스트에 영향을 주지 못 합니다.
• 테스트 파일을 분리할 수 있게 되었으므로, 개발자와 리뷰어 모두 보다 테스트의 관심사에 집중할 수 있습니다. (충돌도 어지간해선 안 날 거라 예상하는데, 중점 확인 파트에서 언급할 내용이 있음.)

리뷰어가 중점적으로 확인해야 하는 부분

1. PR 접두사에 Api:를 붙이지 않은 이유
   • 나머지 UseCase도 리팩토링 작업을 하면, 할 때마다 태그 버전업이 될 텐데 강제로 실패하게 만든 다음 한 번에 태깅하려는 목적이었습니다. 어떻게 생각하시나요??
2. 왜 Service 클래스를 나누다가 말았는지에 대해서. (ex. TargetAmountSaveService가 create와 update 메서드를 가짐)
   • (저장 & 삭제), (수정 & 검색)과 같은 유즈 케이스 조합은 중복 로직이 거의 없다는 것을 가정해보았습니다. (현재로썬 우발적 중복인 경우가 더 많다고 생각합니다.)
   • 하지만 (저장 & 수정), (조회1 & 조회2)의 경우엔 중복 로직이 포함된 경우가 많을 가능성이 높다고 생각합니다.
     • '의존하는 빈도 거기서 거기일 것 같아 합치는 게 클래스 수를 줄이는 측면에서도 합리적이지 않을까?' 라는 생각이었습니다.
     • 다만 이러면 또 테스트 케이스 작성할 때 내부 클래스가 등장해야 할 수도 있습니다. (단, 이전처럼 분리해서 얻는 패널티가 크진 않음)
   • 같은 검색 기능이라도 Actor가 누구냐에 따라 우발적 중복일 수 있기 때문에 분리가 필요하지만, 지금은 딱히 그런 경우는 없는 거 같기도 했어요.
3. Service의 메서드 명이 execute()인 경우
   • 클래스명이 곧 모든 역할을 나타내며, 단 하나의 메서드만을 제공하는 경우입니다.
     • TargetAmountDeleteService.deleteTargetAmount()는 너무 말장난 같아서
   • 직관적이라 좋지 않을까 생각 했는데, 기존 컨벤션에 어긋나서 이야기 해보고 싶어요.
4. Mapper가 UseCase에 위치한 경우와 Service에 위치한 경우.
   • Mapper가 필요한 응답값이 두 개 이상인 경우에 해당합니다. (ex. TargetAmount 정보와 Spending 정보가 모두 필요한 경우)
   • 차라리 TargetAmountSearchService와 SpendingSearchService를 분리하는 게 나을까요?

발견한 이슈

• 없음.

[psychology50]

Mapper 관련해서 TargetAmountSearchService와 SpendingSearchService를 분리하는 게 어떤 의미인지 이해하기 힘드실 수 있을 거 같아 추가 설명하면 다음과 같습니다.

TargetAmountSearchService에서 두 개의 데이터를 모두 조회하려 하지 않고, 서비스를 둘로 나누어 UseCase에서 각각에 대한 요청에 대한 응답을 받습니다.
그 다음 Mapper로 묶어준다면 Service에서 Mapper를 사용하는 걸 방지할 수 있습니다.

[asn6878]

1. PR 접두사에 Api:를 붙이지 않은 이유
2. 왜 Service 클래스를 나누다가 말았는지에 대해서

이해 했습니다. 동의합니다!

3. Service의 메서드 명이 execute()인 경우

궁금한점 하나 남겨두었습니다. 답변해주시면 감사하겠습니다!

4. Mapper가 UseCase에 위치한 경우와 Service에 위치한 경우.

코멘트에서 말씀해주신 대로 분리한다고 하면 Service에서 Mapper를 사용하지 않아도 되는것을 이해했습니다.
하지만 추후에 로직이 복잡해지면 그만큼 분리할 Service도 많아져 복잡성이 올라갈 위험이 있어보입니다.

그러므로 반대로 Service에서 Mapper사용을 방지해야 하는 이유에 대해서 제가 이해가 부족한 것 같아서 조금만 더 설명해주시면 감사하겠습니다 😄

[psychology50]

4. Mapper가 UseCase에 위치한 경우와 Service에 위치한 경우.

코멘트에서 말씀해주신 대로 분리한다고 하면 Service에서 Mapper를 사용하지 않아도 되는것을 이해했습니다. 하지만 추후에 로직이 복잡해지면 그만큼 분리할 Service도 많아져 복잡성이 올라갈 위험이 있어보입니다.

그러므로 반대로 Service에서 Mapper사용을 방지해야 하는 이유에 대해서 제가 이해가 부족한 것 같아서 조금만 더 설명해주시면 감사하겠습니다 😄

일단 Service에서 Mapper 사용을 하지 않게 만드는 것이 주 목적은 아니었습니다.
그저 설계 원칙을 준수했을 때, 자연스레 그리 되는 거라 생각했어요.

현재 목표 금액 조회 시, 반환하는 데이터는 목표 금액과 월 별 지출 내역 총액이 됩니다.
그런데 과연 목표 금액 조회 서비스에서 "지출 내역(Spending)"의 데이터를 호출하는 것이 맞을까? 이 둘을 따로 분리하고 UseCase에서 Mapper를 호출하여 두 개의 이질적인 데이터를 묶어주는 것이 올바르지 않을까? 라는 생각이 들었습니다.
이렇게 하면 단위 테스트 또한 명료해집니다. 목표 금액 데이터가 잘 조회되는지 확인하는데, 월별 지출 내역 총액의 실패가 뜰 일이 없어지기 때문입니다.

혹시 어떻게 생각하시나요??

• 복잡도 문제
  저도 이 문제 때문에 처음에 파사드 패턴을 잘못 적용하고 있는 거였다고 생각합니다. 클래스 수가 너무 많아지는데, 여기에 들어가는 비용이 더 크지 않을 지 우려스러웠으니까요.
  그러나 클래스 수가 늘어나는 게 두려워 잘못된 설계를 한 나머지, 현재 오히려 하나의 클래스에 단일 책임 원칙이 지켜지지 않아 더더욱 프로덕트 코드와 테스트 코드를 작성하는 것에 많은 어려움과 시간이 들고 있다고 생각했습니다.

차라리 관리해야 할 클래스가 조금 늘더라도, 명시적으로 관리할 수 있다면 오히려 이득이 아닐까.....라는 고민 중...
그리고 해봐야 도메인 당 3~4개의 클래스 정도가 생성될 거라 생각해서 나쁘지 않을 것 같기도 했구요!

[asn6878]

현재 목표 금액 조회 시, 반환하는 데이터는 목표 금액과 월 별 지출 내역 총액이 됩니다. 그런데 과연 목표 금액 조회 서비스에서 "지출 내역(Spending)"의 데이터를 호출하는 것이 맞을까? 이 둘을 따로 분리하고 UseCase에서 Mapper를 호출하여 두 개의 이질적인 데이터를 묶어주는 것이 올바르지 않을까? 라는 생각이 들었습니다. 이렇게 하면 단위 테스트 또한 명료해집니다. 목표 금액 데이터가 잘 조회되는지 확인하는데, 월별 지출 내역 총액의 실패가 뜰 일이 없어지기 때문입니다.

혹시 어떻게 생각하시나요??

질문에 답변해주셔서 감사합니다. 계속 느끼는거지만 이번 리팩토링은 테스트를 생각해보면 안보이던 문제들이 보이네요ㅎ

그러나 클래스 수가 늘어나는 게 두려워 잘못된 설계를 한 나머지, 현재 오히려 하나의 클래스에 단일 책임 원칙이 지켜지지 않아 더더욱 프로덕트 코드와 테스트 코드를 작성하는 것에 많은 어려움과 시간이 들고 있다고 생각했습니다.

저도 클래스 개수의 증가 VS 단일 책임 원칙 위배의 임시적 허용 을 기준으로 비교를 해보면 고민 없이 전자를 택할 것 같네요.
블로그 포스팅에도 쓰셨듯 언젠가 또 이 방법의 약점을 찾게 될테니, 그때까지 이렇게 해봐도 좋을 것 같다는 생각입니다~

[psychology50]

블로그 포스팅에도 쓰셨듯 언젠가 또 이 방법의 약점을 찾게 될테니, 그때까지 이렇게 해봐도 좋을 것 같다는 생각입니다~

혹시 "이렇게"가 현상 유지를 말씀하시는 걸까요, 아님 분리한 걸 말씀하시는 걸까요? ㅎㅎ

[asn6878]

혹시 "이렇게"가 현상 유지를 말씀하시는 걸까요, 아님 분리한 걸 말씀하시는 걸까요? ㅎㅎ

분리하는 방법을 써보자는 말이었습니다!

[psychology50]

분리해봤는데, 다시 한 번 리뷰 부탁드리겠습니다~