[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 스타크 미션 제출합니다.

[MODUGGAGI]

체크 리스트

• 미션의 필수 요구사항을 모두 구현했나요?
• Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
• 애플리케이션이 정상적으로 실행되나요?

어떤 부분에 집중하여 리뷰해야 할까요?

안녕하세요!! 아루!! 사이클1에서부터 사이클2를 구현하기까지 상당히 오래 걸린것 같네요.. ㅠㅠ
여러 고민들이 많이 들어서 늦어졌습니다 ㅠㅠ

먼저 아루가 사이클1에서 마지막 리뷰로 남겨주신 부분들에 대해서 적용해본 것으로 시작해보겠습니다!!

사이클1에서의 리뷰 반영

1. switch-case 보다 상차림포맷에서 사용한 것처럼 enum 상수들에게 역할을 부여해보는 건 어때요?

• PieceFormat 클래스에 enum 상수를 만들어서 변환할 수 있게 역할을 부여해보았습니다..!!

2. 개인적인 생각으로는 아직 이 단계는 불필요하다고 생각해요. new JanggiGame(new InputView(), new OutputView())로 적는 것보다 파일도 많고 사용할 메서드도 많은데 구현한 이유는 무엇인지 들어볼까요?

• 의존성을 직접 주입해주는 것보다 설정 클래스를 통해서 한군데에서 관리하게 해주는 것이 더 좋아보여서 다른 크루분의 코드를 보고 적용해보았습니다...
  아루의 리뷰를 보고 사이클2를 시작하기 전에 AppConfig를 제거해서 불필요한 계층을 없앴습니다.
  그런데 사이클2를 진행하는 과정에서 DB 설정을 위한 객체들을 생성하고 Service에 주입해주는 과정에 main 메서드에 추가되었습니다.
  지금과 같은 경우에도 여전히 의존성 주입 클래스를 두어서 의존성을 관리해주는게 너무 과한 단계일까요...??

3. 팀별 상차림을 받기 위해서 Dto를 여기에서 사용하는 게 어색해요. Map으로 Team, ElephantSetUp을 묶는 방향은 어때요? dto 사용하지 않을 수 있을 것 같습니당

• 2개의 각 진영 정보를 담은 DTO를 보내서 같은 진영인지 검증하는 과정을 통해서 중복된 진영 정보를 받는 것을 막게 했었습니다.
• 저는 장기 특성 상 진영이 2개만 존재하기 때문에 그 이상의 개수가 담길 수 있는 Map이 과하다고 생각했었는데... 이런 경우도 Map이 더 나은 걸까요???
  Dto를 제가 항상 너무 쉽게 생각하고 사용하고 있는 걸까욥... ㅠㅠ
• 언제 Dto를 사용하는 것이 좋다고 생각하는지 아루의 생각이 궁금합니다!!

4. 진영 클래스가 점점 비대해지고 있습니다. 리팩토링 과정에서는 본래 어떤 책임이었는지도 집중합시다!
   상차림 관련은 initializer에서 챙길 수 있지 않을까요?

• Camp 클래스는 기존에 아군 진영의 가장 밑단 시작 행, 아군 진영의 전진 방향을 가지고 있는 클래스였습니다.
  Camp 클래스 리팩토링 전

public enum ElephantSetUp {

    public Map<Position, Piece> settingUp(Camp camp) {
        List<Integer> settingColumns = SETTING_COLUMNS;
        if(camp == Camp.CHO) {
            settingColumns = settingColumns.reversed();
        }
        
        //... (로직 생략)
}

Camp 클래스 리팩토링 후

public enum ElephantSetUp {

    public Map<Position, Piece> settingUp(Camp camp) {
        List<Integer> settingColumns = camp.convertElephantColumns(SETTING_COLUMNS);
        
        //... (로직 생략)
    }

}

위 처럼 Camp에 convertElephantColumns() 메서드 추상 메서드로 두어 최대한 분기를 줄여보고자 했습니다.
하지만 이렇게 하면 분기는 줄일 수 있지만 Camp의 역할이 더 늘어나게 됩니다.
Camp가 초기 상차림에 관여해서 어떤 열 정보를 선택할지의 책임이 추가되게 되었습니다.
아루의 말씀을 듣고 곰곰히 Camp의 책임에 대해서 생각해보았습니다.
Camp의 책임은 아군 기물들의 진영 규칙을 담당해야 한다고 생각합니다.
이러한 진영 규칙에는 아군 기물들이 어떤 방향으로 전진하는지, 어느 행에서 시작하는지, 어떤 영역을 자기 궁성으로 보는지처럼 진영 자체의 규칙이라고 보았습니다.

하지만 그 전과 같이 convertElephantColumns(); 를 가지게 되면 진영 별 규칙을 넘어서 초기 상차림 배치 전략까지 관여하게 되어 그 역할 범위를 넘어서게 된다는 것을 알게되었습니다.

그래서 단순 분기를 줄일 수는 있지만 책임을 넘어서게 되는 구조가 된다고 여겨 다시 원래대로 되돌렸습니다.

사이클2 질문

먼저 이번에 사이클2를 적용하면서 요구 사항 중 지키지 못한 부분이 있습니다 😭

1. 인스턴스 변수 최대 2개 제한
2. depth 최대 1단계 제한

1번의 요구 사항은 객체의 책임 분리 측면에서 2번의 요구 사항은 메서드가 하는 일의 단위를 최대한 작게 만들게 하기 위함임을 이해하고 있습니다.
하지만 새로운 요구 사항이 들어오고 DB를 적용하는 과정에서 최대한 지키려고 노력해보았지만 쉽지 않았던 것 같습니다...ㅠㅠ
혹시 객체를 더 분리해서 책임을 잘게 나누고, depth를 더 쪼갤 수 있다면 짚어주시면 다시 한번 도전해보겠습니다!! ㅠㅠ

1. 상수 처리

가장 먼저 궁금한 부분은 상수 처리 문제입니다.
현재 저는 최대한 모든 매직 넘버와 매직 스트링을 클래스 상단에 상수로 선언해서 관리하고 있습니다.
그런데 이렇게 하다보니 특정 클래스는 상수 개수가 너무 많아져서 제 개인적인 입장에서는 오히려 이해하기가 힘들게 느껴졌습니다.

제가 이렇게 모든 매직 넘버와 스트링을 상수 처리 하는 경험이 아직 없기 때문에 코드를 볼 때 낯설음 느끼는 것인지, 아니면 실제로 제가 너무 과하게 상수 처리를 하고 있는 것인지 궁금합니다..!!

2. 행마법의 책임 분리

저는 행마법을 2가지로 나누어서 보았습니다.

1. 해당 기물이 이동할 수 있는 경로(MoveStrategy)
2. 해당 기물이 이동하기 위한 조건(MoveCondition)

이렇게 나누어서 저는 Piece 객체가 각각 2개 이동 담당 객체에게 메시지를 보내고 응답의 결과를 종합해서 자신의 외부로 전달하며 기물 이동 여부를 검증하는 책임을 다할 수 있다고 보았습니다.

그런데 다른 크루분들의 코드를 보니 MoveCondition을 이렇게 따로 전략으로 빼내지 않고 MoveStrategy 안에 메서드로 아예 박아놓은 것을 보았습니다.

이처럼 MoveStrategy 1개의 클래스에 모든 전략을 다 넣어놓는 것이 1개의 클래스에서 이동과 관련된 모든 것을 관리하니 응집도가 더 높아지고 좋은 방식인걸까요..??
저는 제 방식 처럼 2개의 클래스가 모여 행마법을 구성하게 하면 순수 이동 경로 탐색, 이동 조건 탐색으로 나누어 테스트 단위도 더 잘 쪼갤 수 있고, 공통 부분을 빼낼 수 있어서 좋은 구조라고 생각했는데

이렇게 2개로 나눠놓은 것이 오히려 과도한 추상화를 한 것일까요..?

3. 궁성 구현

제가 초기에는 궁성 구현에 있어서 다음과 같은 구조를 생각했습니다.

하지만, 궁성은 이동 전략의 한 종류라기보다 보드가 가지는 규칙에 가깝다고 판단했습니다.
이를 전략 계층의 상위 추상화로 두게 되자 "1칸 이동", "여러 칸 이동"처럼 기물의 행마법(하위 전략)이 상위 계층인 궁성 전략에 묶이는 구조가 되는 것 같았습니다.

그래서 이번 구현에서는 궁성 전략을 전략으로 두지 않고 각 이동 전략이 자신의 이동 규칙을 유지한 채 궁성 관련 판단만 필요한 곳에서 직접 처리하는 방향을 선택했습니다.

하지만 이렇게 구현하고 나니, 새로운 요구사항인 "궁성 영역의 추가"가 들어왔을 때 제 코드에서는 확장이 일어나지 않고 변경이 일어났다고 느껴졌습니다.
이 경우 비즈니스 규칙 자체가 변경된 것이므로 기존 코드의 수정이 자연스러운 것인지, 아니면 구조를 더 잘 나누었다면 궁성 영역의 처리도 확장 하는 것으로 대응이 가능했을 지 궁금합니다.

4. GameService의 테스트

서비스 계층 테스트를 작성해본 경험이 없어서, 현재 제가 작성한 GameService 테스트 방식이 적절한지 궁금합니다.

현재는 GameService가 단순 계산보다는 트랜잭션과 Repository를 조합해 게임을 생성/조회/저장하는 역할을 하기 때문에 테스트 db를 통해 이러한 기능들을 잘 수행하는지를 테스트했습니다.

그런데 이런 테스트가 GameService의 책임을 검증하는 데 적절한지 아니면 이런 경우 테스트 해야하는 별도의 방식이 있는지 궁금합니다..