[🚀 사이클1 - 미션 (보드 초기화 + 기물 이동)] 무빙 미션 제출합니다.

[mvg01]

개요

안녕하세요 찰리, 무빙입니다.
이번 사이클 1은 저에게 꽤 어렵게 다가온 미션이었습니다. 구현을 진행하는 내내 설계에 대해 오래 고민했고, 페어와 많은 대화를 나누며 최대한 객체지향적인 방향을 지키고자 노력했습니다.
다만 아직 자바와 객체지향에 대한 이해가 부족하다 보니, 기물 이동 관련 클래스와 메서드에는 중복이 남아 있고, 각 기물의 움직임 전략을 초기화하는 부분도 다소 길고 복잡해졌습니다. 이번 단계에서는 우선 완전히 동작하는 코드를 만드는 것을 목표로 두고, 최소한의 리팩토링만 진행한 상태로 PR을 드립니다. 부족한 부분은 리뷰를 통해 차근차근 개선해보고 싶습니다.

현재는 게임의 턴 진행, 궁성, 승패 로직까지는 구현하지 못했습니다. 차림법을 입력받아 양 팀의 기물을 초기화하고, 보드를 구성해 출력하는 흐름까지 컨트롤러에서 담당하고 있습니다.
제가 객체지향 개념과 자바스킬이 많이 부족합니다. 그만큼 이번 리뷰에서 많이 배우고 싶습니다. 편하게 여러 관점에서 피드백 주시면 감사하겠습니다. 환절기 건강 유의하세요!

체크 리스트

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

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

1. 보드 상태를 어디에서 관리해야 하는지에 대한 설계

가장 크게 고민한 부분입니다. 사전학습과 토론 과정에서도 기물이 보드의 상태를 직접 알 책임이 없다고 생각했고, 기물은 자신의 이동 규칙만 알고 있어야 한다고 생각했습니다. 하지만 실제로 이동 가능한 위치를 계산하는 과정에서는 보드 상태가 계속 필요했고, 이를 기물에 직접 전달하다 보면 기물이 보드를 알게 되는 구조가 되어 불편했습니다.

그래서 보드와 기물 사이에 BoardMediator를 두고, 기물은 보드의 구체적인 구현을 모르더라도 필요한 정보만 받아올 수 있도록 설계했습니다. 이 방식이 캡슐화를 유지하는 방향인지, 혹은 오히려 중재자인 BoardMediator에게 책임이 과하게 몰리는 구조인지 피드백을 받고 싶습니다. 찰리라면 어떻게 기물과 보드의 상태를 관리했을지도 듣고 싶습니다.

2. movement를 1칸 단위로 다루는 현재 방식이 적절한지

처음에는 movement가 한 번에 여러 칸을 표현할 수 있도록 maxDistance 개념을 두고 싶었습니다. 예를 들어 상의 경우도 “대각선 1칸을 두 번”이 아니라 “대각선 방향으로 최대 2칸”처럼 표현하고 싶었습니다. 하지만 구현 과정에서 복잡도가 많이 올라가서, 현재는 모든 movement를 사실상 1칸 단위로만 처리하고 있습니다(위 부분은 코드로 코멘트 남기겠습니다.)

이 방식이 오히려 더 단순하고 유지보수에 유리한지, 아니면 처음 의도했던 것처럼 movement 자체가 거리 개념을 갖는 편이 더 나은지 조언을 듣고 싶습니다.

3. 기물 초기화와 전략 초기화에서 new가 많이 등장하는 구조

각 기물의 pieceAction 초기화, 그리고 보드의 초기 배치 과정에서 객체를 직접 생성하는 코드가 많이 들어갔습니다.
지금은 동작을 우선시해서 작성했지만, 작성하면서도 다소 하드코딩처럼 느껴졌습니다.

현재 설계 안에서 이 부분을 더 간결하게 줄일 수 있는 방법이 있을지, 혹은 팩토리나 다른 구조로 분리하는 것이 자연스러운지 피드백을 받고 싶습니다.

4. 테스트 범위와 테스트 케이스를 어디까지 가져가야 하는지

이번 구현에서 테스트를 작성하는 데도 많은 시간이 들었습니다. 특히 이동 규칙은 경우의 수가 많아, 어디까지 테스트해야 충분한지 계속 고민하게 되었습니다.
모든 경우를 끝까지 집요하게 추적해 완전무결하게 테스트하는 것이 맞는지, 아니면 핵심 규칙과 대표 케이스 중심으로 가져가는 것이 더 현실적인지 기준이 궁금합니다.

또한 테스트를 작성하다 보니 fixture 중복이나 케이스 설계에도 시간이 많이 들었는데, 이런 부분에 대한 팁도 함께 듣고 싶습니다. 팀에서도 테스트는 비즈니스 로직이 있는 public 메서드 중심으로 작성하고, fixture 로직이 반복되면 별도 유틸로 분리하는 기준을 정리해두었는데, 제가 실제 코드에 그 기준을 잘 반영하고 있는지도 봐주시면 감사하겠습니다.

[Gomding]

안녕하세요 무빙!
피드백 잘 반영해주셨네요 👍
몇가지 코멘트 남겼으니 확인부탁드려요~
궁금한 점 있으면 언제든 DM 이나 코멘트 남겨주세요 :)

[Gomding]

굉장히 깔끔하네요!

[Gomding]

요런 부분은 Controller 에 있을 흐름이 아닌것 같네요 🤔
도메인 쪽에 흐름을 만들어보는것은 어떻게 생각하시나요?

[Gomding]

해당 책임이 Movement 에 있어도 괜찮을지 조금 의문이 들었어요 🤔
무빙은 어떻게 생각하시나요?

[Gomding]

Movement 클래스 전체적으로 메서드 길이나 depth1 규칙을 체크해보시면 좋겠어요!

[Gomding]

미리 공유해주셔서 감사해요~ 👍

[Gomding]

취향 차이이지만 부정문인 조건보다 긍정문으로 읽을 수 있게 배치하는것도 가독성을 높이는 방법이라 생각해요~

Suggested change

	if (!hasPieceIn(position)) {
	throw new IllegalArgumentException("요청된 위치에는 기물이 존재하지 않습니다.");
	}
	return positionPieceMap.get(position);
	if (hasPieceIn(position)) {
	return positionPieceMap.get(position);
	}
	throw new IllegalArgumentException("요청된 위치에는 기물이 존재하지 않습니다.");

[Gomding]

이건 view 와 매우 깊은관계라 view 에서 변환하면 될 것 같은데 무빙은 어떻게 생각하시나요!
DTO 가 view 의 영역까지 너무 깊게 알고있다고 느껴져서요 :)

[Gomding]

BoardDto 전체적으로 DTO 의 책임범위를 넘어서는 부분이 있는것 같아요 🤔
무빙은 어떻게 생각하시나요?

[Gomding]

모든 piece 들이 PieceAction과 PieceType이 필요한데 Piece 를 추상 클래스로 만들거나 Piece 하위에 추상클래스로 계층 하나를 두는것도 방법이겠어요 :)

[Gomding]

아래와 같이 표현하면 상수의 값 수정 시 수정할 위치가 줄어드니 편할 것 같네요 :)

Suggested change

	throw new IllegalArgumentException("행 입력은 1~10을 입력해야 합니다.");
	throw new IllegalArgumentException("행 입력은 %s~%s을 입력해야 합니다.", MINIMUM_ROW, MAXIMUM_ROW);