[레거시 코드 리팩터링 - 1단계] 수달(허수진) 미션 제출합니다 #205
Conversation
| > ## 수달이 이해한 도메인 ✍🏻 | ||
| 키친 포스는 오프라인 주문을 받을 수 있는 서비스에요. | ||
| 오프라인 기준으로는 매장에서 주문을 직접 받고 주문에 대한 상태관리를 해요. | ||
| 상태에는 주문이 후 조리가 시작되고, 손님이 식사를하고, 계산하는 순서가 있겠죠? |
There was a problem hiding this comment.
리드미 작성이 너무 친절하네요.. 이런 꼼꼼한 점은 저도 배워가고 싶어요! 💯💯
| 매장에서는 특수하게 한 사람이 여러 테이블을 계산할 수도 있어요. 더즈가 팀장이 되면 | ||
| 금요일 주말에 스모디 사람들에게 한턱 쏠때 처럼요. ^^ 그 때 관리를 하기 위한 테이블 그룹이라는 도메인도 있다는 점이 신기하네요. |
| public enum ExceptionType { | ||
|
|
||
| NOT_FOUND_MENU_EXCEPTION("없는 메뉴가 포함된 주문입니다."), | ||
| NOT_FOUND_TABLE_EXCEPTION("없는 테이블에 대한 요청입니다."), | ||
| INVALID_CHANGE_ORDER_STATUS_EXCEPTION("이미 완료된 주문입니다."), | ||
| INVALID_PROCEEDING_TABLE_GROUP_EXCEPTION("진행중인 테이블 그룹이 존재합니다."), | ||
| INVALID_PRODUCT_PRICE_EXCEPTION("유효하지 않은 상품 가격입니다."), | ||
| INVALID_TABLE_GROUP_EXCEPTION("그룹 구성은 두개 이상의 테이블 부터 가능합니다."), | ||
| INVALID_TABLE_UNGROUP_EXCEPTION("아직 그룹을 해지할 수 없습니다."), | ||
| INVALID_CHANGE_NUMBER_OF_GUEST("게스트 숫자는 음수일 수 없습니다."); | ||
|
|
There was a problem hiding this comment.
수달네 프로젝트에서도 이렇게 예외 관리를 하셨나요? 저희 팀에서도 하나의 비즈니스 예외 클래스에 내부 상태로 여러 상황들을 넣는 식으로 관리했어요.
이렇게 하니까 커스텀 예외 갯수를 작게 유지하면서 여러 상황을 표현해서 클라이언트에게 전달할 수 있다는 장점이 있는 것 같아요 👍
| @SpringBootTest | ||
| public class ServiceTest { | ||
|
|
||
| @MockBean | ||
| protected MenuDao menuDao; | ||
| @MockBean | ||
| protected MenuGroupDao menuGroupDao; | ||
| @MockBean | ||
| protected MenuProductDao menuProductDao; | ||
| @MockBean | ||
| protected ProductDao productDao; | ||
| @MockBean | ||
| protected OrderDao orderDao; | ||
| @MockBean | ||
| protected OrderLineItemDao orderLineItemDao; | ||
| @MockBean | ||
| protected OrderTableDao orderTableDao; | ||
|
|
There was a problem hiding this comment.
현재 각 도메인마다 다른 Mock 객체를 사용해서 중복되는 객체도 있고, Application Context 가 여러번 뜨고 있었어요. 그 문제를 해결하기 위해서 부모 객체를 만들어 상속구조로 중복 제거와 Context 를 재사용하도록 변경해두었습니다.
서비스 레이어를 테스트할 때 통합 테스트를 진행할 수도 있고 수달처럼 DAO 계층을 모의해서 슬라이스 테스트를 할 수도 있다고 생각합니다. 이건 테스트하려는 개발자의 의도와 관련있다고 생각해서 정답이 있다고 생각되지는 않아요.
그리고 애플리케이션 컨텍스트를 재사용하기 위해서 부모 클래스로 통합 처리한 방법은 좋은 방법이라고 생각해요 저도 즐겨쓰는 방법입니다.
그런데 이왕 목 객체를 사용한다고 했으면 과연 @SpringBootTest가 필요한지 의문이기는 해요. 사실 mock만 제대로 주입해 준다면 컨텍스트를 띄울 필요가 없기도 하고요.
@ExtendWith(MockitoExtension.class)
public class ServiceTest {
@Mock
protected MenuDao menuDao;
@Mock
protected MenuGroupDao menuGroupDao;
@Mock
protected MenuProductDao menuProductDao;
@Mock
protected ProductDao productDao;
@Mock
protected OrderDao orderDao;
@Mock
protected OrderLineItemDao orderLineItemDao;
@Mock
protected OrderTableDao orderTableDao;
저라면 컨텍스트를 띄우지 않고 위와 같이 Dao의 목 객체들을 세팅해 놓고 @SpringBootTest 없이 슬라이스 테스트를 진행해서 테스트 속도를 개선해볼 수도 있을 것 같아요. 수달은 어떻게 생각하나요?
ServiceTest를 상속받은 테스트에서 아래처럼 mock을 주입하면 테스트는 돌아갈 겁니다.
스프링을 사용하고 있지만 과연 @SpringBootTest가 꼭 필요한지, 없이 할 수 있다면 없이 하는 것도 방법이라 생각해요!
| protected void 완료된_주문_조회() { | ||
| Mockito.when(orderDao.findById(anyLong())).thenReturn(Optional.of(주문_생성(OrderStatus.COMPLETION))); | ||
| } | ||
|
|
||
| protected void 메뉴존재유뮤세팅(Long count) { | ||
| Mockito.when(menuDao.countByIdIn(any())).thenReturn(count); | ||
| } | ||
|
|
||
| protected void 존재하지않는_테이블_세팅() { | ||
| Mockito.when(orderTableDao.findById(anyLong())).thenReturn(Optional.empty()); | ||
| } | ||
|
|
||
| protected OrderTable 테이블_생성(Long id) { | ||
| return new OrderTable(id, 1L, 1, false); | ||
| } | ||
|
|
||
| protected void 테이블_그룹이_없는_테이블_세팅(Long id) { | ||
| Mockito.when(orderTableDao.findById(anyLong())).thenReturn(Optional.of(new OrderTable(id, null, 1, false))); | ||
| } | ||
|
|
||
| protected void 존재하는_테이블_세팅() { | ||
| Mockito.when(orderTableDao.findById(anyLong())).thenReturn(Optional.of(테이블_생성(1L))); | ||
| } |
There was a problem hiding this comment.
서비스 레이어의 내부 로직이 드러나지 않도록 한 번 감춘 코드라고 하셨는데 가독성 면에서도 크게 좋아진 것 같아요! 👍
안녕하세요 더즈 수달 입니다.😀
미션을 하면서 마치 처음 회사에 가서 도메인 파악을 하는 그런 느낌을 받았습니다. ㅎㅎㅎ
요구사항 도출하는데 시간이 조금 걸렸어요. 더즈도 이해하기 어려우실 것 같아서
Javadoc활용해서 Test 마다 작업 내역에 대한문서화 해두었습니다.
그 문제를 해결하기 위해서 부모 객체를 만들어 상속구조로 중복 제거와 Context 를 재사용하도록 변경해두었습니다.
내부 로직이 테스트 코드에 들어난다는 점에서 마음에 들지 않더라고요. 그래서 한번 추상화를 해서 상위 객체에 관련 로직을 둠으로 캡슐화를 시도해보았어요.
현재 검증 로직의 대부분이
IllegalArgumentException예외를 반환하고 있는데, 테스트에서 구체적으로 어떤 내용에 대한 예외인지명시해주고 싶어서 메세지를 담게 되었어요. 표준 예외에 메세지를 담으려고 하니까 ExceptionType.getMessage() 와 같이 데이터를
꺼내야하는 일이 중복해서 발생하고 있었어요. 따라서 ExceptionType 이라는 예외관리하는 객체와 협력하는 객체를 하나 만들어서 위 작업을 진행하면 재밌겠다는 생각에 만들어보았어요.
Test 에 대한 내용이 머지되면 빠르게 서비스 계층에 있는 레거시들을 각자 책임으로 돌려놓는 작업을 할 예정입니다.
그래서 2단계 들어가면 테스트 구조나 내용이 바뀔 수 있을 것 같아요. ㅎㅎㅎ 이점 참고해서 리뷰 주시면 감사하겠습니다.