[1, 2단계 - 체스] 져니(이지원) 미션 제출합니다.

[Cl8D]

안녕하세요, 서브웨이! 5기 백엔드 크루 져니입니다 ☃️
이번 미션은 너무 부족한 점이 많아서 리뷰 요청 드리기도 민망하네요... 💦
미션을 진행하면서 명령어에 대해 상태 패턴을 적용하여 시작, 움직임, 종료 상태를 관리한 덕분에 컨트롤러의 코드를 가볍게 가져갈 수 있었어서 이 부분은 뿌듯했던 것 같아요!
다만, 체스 게임을 관리하는 ChessGame 클래스가 하는 일이 많고, 굳이 해당 말이 폰인지 나이트인지 확인하는 과정이 들어가는 게 너무 절차지향적이라는 생각이 들어서 이 부분은 어떤 식으로 수정하면 좋을지 조언을 듣고 싶어요! (책임 자체의 분리가 문제인 것 같은데... 쉽게 고쳐지지 않네요 ㅠㅠ) 체스판에 체스 말이 존재하는지 확인하기 위해서 chessBoard를 확인하려다 보니까 chessGame이 아닌 각 말의 xxxMove에서 구현하기가 어려운 것 같아요...! ㅠ_ㅠ

또한, 일급 컬렉션 사용이나 컨벤션 등을 아직 지키지 못해서, 그 부분은 다음 단계를 진행하면서 함께 수정해두도록 하겠습니다!
부족한 코드지만 잘 부탁드립니다! 🙇‍♀️
+) 코드는 PR 이후에도 계속해서 수정해나가도록 하겠습니다!

[Cl8D]

안녕하세요, 서브웨이! 리뷰해주신 내용을 바탕으로 리팩터링을 진행했어요!
자잘한 부분은 제외하고, 크게 변경한 부분은 다음과 같아요 :D

• Move 클래스에 대한 테스트 추가
• 도메인과 뷰의 의존성을 끊어주기 (Dto 활용)
• 체스말에 대한 구조 변경

---

✔️ 도메인과 뷰의 의존성을 끊어주기

• 이때, PieceName에서 사용된 enum 타입 (PieceType, CampType)에 대해서도 Dto를 생성해야 되는지 고민이 있었는데, 다음 레퍼런스를 보고 이에 대해서는 처리하지 않도록 결정했습니다! 괜찮은 방향일까요?

• 위 레퍼런스에는 enum 타입의 경우 상수의 모음이기 때문에 상관없다고 하고 있어요! 저도 같은 생각이기도 하고, 두 곳 모두에서 동일한 상수를 정의한다고 한다면 도메인 쪽 상수가 변경되었을 때 뷰의 상수도 함께 변경해줘야 해서 오히려 변경 지점이 더 커지는 것이 아닐까… 라는 생각이 들었어요. 혹시 서브웨이는 어떻게 생각하시나요?

• 다만, Dto로 변환하다 보니까 getter의 사용은 필수적이라고 보는데, 본 미션의 요구사항인 ‘getter의 지양’은 뷰의 출력을 위해 사용하는 것에는 포함되지 않는다고 생각해도 될까요?

---

✔️ 체스말에 대한 구조 변경

• 기존에는 각각의 체스말에 대한 구체 클래스를 생성했었지만, 리팩터링을 통해서 Movable 필드를 조합으로 가지게 하여, 외부에서 해당 체스말이 어떤 식으로 이동할 수 있을지 이동에 대한 전략을 주입받도록 수정했습니다! (다만, 인스턴스 필드의 수가 3개가 된다는 게 조금 마음이 걸리지만, 이 이상으로 줄이는 건 불필요한 분리가 될 것 같아서 우선은 이렇게 두었습니다…!)

• 덕분에 Knight - KnightMove에서 진행한 테스트의 중복을 없앨 수 있었고, 폰의 움직임 전략을 ChessBoard가 아닌 Piece 자체가 결정할 수 있게 되어서 조금 더 가벼워진 것 같습니다! (이전에 변경 요청 해주신 게 이런 방향이 맞는지는 모르겠지만… 맞을까요?? 🤔)

---

이번에도 리뷰 잘 부탁드립니다! 감사합니다 🙇‍♀️

[Cl8D]

리뷰가... pending 상태였다는 것을 뒤늦게 알아서 뒤늦게... submit을 해보았습니다.......!! (리뷰가 와다다 달린 느낌인데.... ㅠ_ㅠ 다음부턴 신경쓰겠습니다!)

[joseph415]

안녕하세요 져니!

리뷰 잘 적용해주셨네요💯
추가 코멘트 적어두었습니다. 확인한번 부탁드립니다!

[joseph415]

개인적으로는 dto로 변경하는 로직 조차도 엄격하게 도메인을 모르게만들고싶은데,,ㅎ 요거는 제 욕심이라 의견으로만 남겨둡니다!

[Cl8D]

서비스 레이어를 두면 좋을 것 같은데, 그런 의도로 말씀 주신 거 맞을까요~?! 생성해두겠습니다 ㅎㅎㅎ 👍
+) 그런데 혹시, dto 역시 도메인을 모르게 하는 이유가 있을까요? 사실 dto는 도메인의 필드값들과 밀접하게 관련이 있을 수밖에 없기 때문에 도메인에 대한 업데이트가 발생하면 dto도 변경이 발생하는 게 당연하다고 생각이 들었거든요!

아니면, 이런 이유일까요?
[dto가 도메인을 알고 있는 경우]

1. 도메인 - dto 작성 후 1차 배포
2. 2차 배포 시 도메인 영역에 대한 수정이 발생함 (ex. 필드 제거) -> 도메인 수정 후 배포
3. dto가 도메인 자체를 참조하고 있을 경우, 해당 dto에서도 변경 발생

[dto가 도메인을 모를 경우 - 필드로 각 요소를 주입받음]

1, 2번 동일
3. dto가 주입받은 필드가 도메인에서 사용되는 필드였음 
-> 서비스에서 해당 필드 주입 제거 전까지는 dto가 완전한 상태임

제가 제대로 이해한 게 맞는지 궁금합니다!

[joseph415]

제가 의도한거는 서비스를 만들라는 것은 아니였고, controller 에서 직접 new BoradDto(..) 선언해서 사용하는 것을 말씀드린거에요!
아니면 Mapper class를 만들어서 해당 클레스에서 Domain을 받고 Dto를 리턴해주는 형태로사용하던가요!

[joseph415]

그리고, 지금 서비스를 잘못사용하고 계신것 같습니다. 현재 서비스를 dto를 만드는 factory처럼 사용하고 계시는데요ㅎㅎ.. 서비스의 역할은 이게 아닙니다.

서비스의 역할을 찾아보시면 좋을 것 같아요.

서비스는 비지니스 로직의 흐름을 담당하는 클래스입니다. dto를 만드는 factory 역할을 하는게 아닙니다!
원래는 서비스가 없다면 Controller에서 비지니스로직을 호출하여 사용하게 될텐데요. 애플리케이션이 커지면, 복잡해지기 때문에 이런 흩어진 로직을 하나로 묶어서 고수준의 형태로 추상화해서 사용하는 것이 서비스입니다.

따라서 비지니스 로직의 흐름을 고수준으로 묶기 때문에 서비스의 메서드는 하나의 applicaiton 의 기능을 하게 되는것이죠.

따라서 해당 구현들은 지금 잘못된것 같아요. 네이밍을 그냥 Mapper 기능을 하고 있어서 네이밍만 변경해서 사용하시면될 것 같아요.
그리고 Mapper의 utils 성 클래스이기 때문에, 인스턴습변수도 필요 없어질 것 같습니다

[joseph415]

dto 는 view <-> controller 사이에서 도메인의 의존을 끊기 위해서 사용하는 것인데, dto가 도메인을 알면 사용하는 의미 자체가 퇴색되는것 같아요.

이런 의존성을 끊어주는 이유는 서로의 변경사항을 직접적으로 영향받지 않도록 하기 위함입니다.

domain이 변경돼서 view가 직접적으로 영향을 받거나, view의 변경으로 domain이 직접적으로 영향을 받거나 하는 상황이요.
도메인이 변경돼서 view가 아니라 dto가 변경이되는거면 의존성을 잘 끊어준것이 되겠죠?

만약 만약에

1. dto로 도메인엔티티 전체가 아니고 일부만 내보내고 있다고 가정해볼게요. 필요한거는 일부분인데 굳이 필요하지 않은 변경사항때문에 재배포를 해야할 수 도 있습니다.
2. 도메인은 변화에 유연하게 대처해야합니다. 만약에 도메인이 그대로 외부로 나가게 됐는데, 해당 도메인의 전체를 갈아 엎어야하는경우가 생기게 됐는데, 사용자가 버전업데이트를 하지 않으면, 해당 버전을 사용하는 사용자를 위해서 변경해야할 도메인은 변경하지 못합니다. (변경해버리면, 해당 사용자는 버그가 나겠죠?, 이 때문에 레거시가 계속 남게됩니다.)
3. 의존성을 잘 관리하는 이유는 각 영역간의 영향성을 최소화 하기 위함입니다. view는 사용자의 영역이고, domain 은 가장 중요한 비지니스 영역인데 이 둘이 의존성이 엮어서 서로에게 영향을 줘야할 이유가 없습니다!
4. 아래 이유

이동욱님 블로그 글을 참고로 달아둘게요.

여기서 Entity 클래스와 거의 유사한 형태임에도 DTO 클래스를 추가로 생성했는데요.
절대로 테이블과 매핑되는 Entity 클래스를 Request/ Response 클래스로 사용해서는 안됩니다.
Entity 클래스는 가장 Core한 클래스라고 보시면 되는데요.
수많은 서비스 클래스나 비지니스 로직들이 Entity 클래스를 기준으로 동작합니다.
Entity 클래스가 변경되면 여러 클래스에 영향을 끼치게 되는 반면 Request와 Response용 DTO는 View를 위한 클래스라 정말 자주 변경이 필요합니다.

이런 이유들은 지금같은 콘솔프로그램같은 작은 서비스에선 와닿지 않을거에요!.. 이해가 안되더라도 경험하시면서 이해하도 좋을 것 같아요..

[Cl8D]

서브웨이의 의견을 읽고 제가 이해한대로 말씀을 드려도 괜찮을까요?

1. 서비스의 역할에 대해
서비스는 “비즈니스 로직”을 담는다.
이유 -> 컨트롤러에서 호출하기 위한 비즈니스 로직들을 모아서 관리하기 위해 + 추상화 목적

여기서 말씀해 주신 추상화의 목적은 다음과 같을까요?
ex) 주문 관리 서비스

1. OrderService라는 하나의 주문 서비스에 대한 인터페이스 생성
2. 식품에 대한 FoodOrderService, 옷에 대한 ClothOrderService가 구현체로 있다.
3. 두 서비스는 접근하는 엔티티가 다르다. 전자는 Food 엔티티, 후자는 Cloth 엔티티에 대해서 접근한다.
4. 단순히 주문 자체에 대한 api는 주문 서비스에 대해서만 알고 있으면 된다. 추상화된 OrderService를 활용하여 주문을 진행한다.
   이런 식으로 생각하면 될까요?

2. dto에서 도메인을 알면 안 되는 이유
아래에서 설명해주신 이유는 뷰가 도메인 자체를 의존하면 안 되는 이유라고 생각해요. 그래서 dto를 사용하는 것까지는 저도 동의하는 바예요. (특히 웹 서비스로 가게 된다면 당연히 엔티티의 모든 내용을 내려주는 건 안 되니까 dto를 통해서 반환하는 것이겠구요. 순환 참조의 문제도 있을 것이구요.)

다만, 제가 궁금한 건 dto로 변환하는 로직에서, dto가 도메인을 자체를 가지고 변환 작업을 하면 안 되는 이유가 궁금했어요. 서브웨이는 dto도 일종의 뷰의 영역으로 보고 말씀하시는 것일까요?…
그렇다면 도메인의 변화가 생겼을 때 뷰와 밀접하게 관련이 있는 dto가 바로 영향을 받도록 하지 않고, 한 단계를 두어서 변화를 최대한 받지 않도록 하기 위해서라고 보면 될까요??…

[Cl8D]

우선 서비스가 dto를 변환하는 건 옳지 않다는 것까지는 이해했어요! 다만... 별도의 매핑 클래스까지 두는 게 맞을까? 라는 의문이 들었습니다!

[joseph415]

1번

네 맞습니다.

서비스가 없으면 컨트롤러에서 세부 구현들이 들어가겠죠!

controller {

orderFood() {
1. 주문 객체를 만든다.
2. 주문과 관련된 비지니스 로직들을 처리한다.
3.  ...
4. ...
}

orderCloth() {
1. 주문 객체를 만든다.
2. 주문과 관련된 비지니스 로직들을 처리한다.
3.  ...
4. ...
}
}

이런 행위들이 분산될 거고, 이걸 하나의 기능으로 추상화해서 사용하기 위해 서비스를 사용합니다. 따라서 서비스 하나의 애플리케이션 기능을 나타낸다고 할 수 있고, 비지니스 로직의 흐름을 담당한다고 할 수 있습니다.

2번

public static BoardDto from(final Map<Position, Piece> board) 이처럼 변환하는 로직 자체를 왜 알지 못하게 해야하는가? 에대한 질문인가요?

저는 dto에 대해서 이러한 생각을 갖고 있습니다.

1. dto는 data trasnfer object로 view에 필요한 데이터를 객체로 넘겨주기 위해 사용하는 역할이기 때문에 dto에는 getter,setter 이외에 어떤 로직, 어떤 의존성도 안들어갔으면 합니다.
2. 위와 같은 이유로 dto는 view에 필요한 데이터를 넘겨주기만 하면 되는데, 굳이 불필요한 도메인 의존이 들어가야 하나? 라는 생각이 듭니다. (dto와 도메인 패키지간의 결합이 생기는 것이 불필요한 의존이라고 생각해요!)
3. 그리고 사실 변환하는 책임은 컨트롤러의 책임이 아닌가생각합니다!
4. 마지막으로 매핑클래스를 두는것은 컨트롤러에서 가공하는 역할들이 많아지게 됐을 때, Mapper 클래스를 둬서 컨트롤러의 책임을 분배시도록 할 수 있다고 생각합니다.

domain의 영향이 dto에 영향을 끼쳐서라기보단, 위와 같은 이유로dto가 domain을 알아야 하나 라는 생각을 했습니다.

현재 변환로직이 단순 생성자로 변환하는것 말고는 따로 없고, 변환로직이 들어가는 것에 대해 어떻게 생각하느냐에 따라 달라질 것 같아서 그냥 의견으로 드렸던 것입니다!

만약 변환하는 메서드가 로직을 갖고 있었다면, 변경해달라고 리뷰를 하긴했을 것 같네요!

[joseph415]

커멘트가 시간 순서대로 써진게 아니여서 굉장히 헷갈리네요ㅎㅎ; 이전 리뷰에도 답변 달아둔거는 resolve 안눌렀습니다 한번 확인부탁드려요!

(그래서 삭제하고 다시 달았습니다!)

[Cl8D]

안녕하세요, 서브웨이! 이번에는 크게 수정할 게 많이 없어서 금방 수정 완료했습니다 ㅎㅎ
도메인과 dto의 의존을 완전히 끊기 위해서 서비스 레이어를 도입했어요!
다만, 아직까지는 dto가 도메인 자체를 완전히 아는 게 어떤 문제가 있는지 확실하게 와닿지 않는 것 같아요...!

혹시 실제 예시나, 조금 더 구체적인 예외 사항을 알려주실 수 있을까요?
이전 커멘트에도 답글을 달아두었으나, 제가 제대로 이해한 게 맞는지 모르겠어서 🥲
이번 리뷰도 잘 부탁드립니다!!

[joseph415]

안녕하세요. 져니

리뷰 잘 적용해주셨네요💯
질문을 커멘트로 달아두었고, 간단한 리뷰 몇가지 달아두었습니다.
다음단계를 위해서 merge할게요! 해당 리뷰는 다음단계에서 적용해주세요!

[joseph415]

제가 의도한거는 서비스를 만들라는 것은 아니였고, controller 에서 직접 new BoradDto(..) 선언해서 사용하는 것을 말씀드린거에요!
아니면 Mapper class를 만들어서 해당 클레스에서 Domain을 받고 Dto를 리턴해주는 형태로사용하던가요!

[joseph415]

그리고, 지금 서비스를 잘못사용하고 계신것 같습니다. 현재 서비스를 dto를 만드는 factory처럼 사용하고 계시는데요ㅎㅎ.. 서비스의 역할은 이게 아닙니다.

서비스의 역할을 찾아보시면 좋을 것 같아요.

서비스는 비지니스 로직의 흐름을 담당하는 클래스입니다. dto를 만드는 factory 역할을 하는게 아닙니다!
원래는 서비스가 없다면 Controller에서 비지니스로직을 호출하여 사용하게 될텐데요. 애플리케이션이 커지면, 복잡해지기 때문에 이런 흩어진 로직을 하나로 묶어서 고수준의 형태로 추상화해서 사용하는 것이 서비스입니다.

따라서 비지니스 로직의 흐름을 고수준으로 묶기 때문에 서비스의 메서드는 하나의 applicaiton 의 기능을 하게 되는것이죠.

따라서 해당 구현들은 지금 잘못된것 같아요. 네이밍을 그냥 Mapper 기능을 하고 있어서 네이밍만 변경해서 사용하시면될 것 같아요.
그리고 Mapper의 utils 성 클래스이기 때문에, 인스턴습변수도 필요 없어질 것 같습니다

[joseph415]

dto 는 view <-> controller 사이에서 도메인의 의존을 끊기 위해서 사용하는 것인데, dto가 도메인을 알면 사용하는 의미 자체가 퇴색되는것 같아요.

이런 의존성을 끊어주는 이유는 서로의 변경사항을 직접적으로 영향받지 않도록 하기 위함입니다.

domain이 변경돼서 view가 직접적으로 영향을 받거나, view의 변경으로 domain이 직접적으로 영향을 받거나 하는 상황이요.
도메인이 변경돼서 view가 아니라 dto가 변경이되는거면 의존성을 잘 끊어준것이 되겠죠?

만약 만약에

1. dto로 도메인엔티티 전체가 아니고 일부만 내보내고 있다고 가정해볼게요. 필요한거는 일부분인데 굳이 필요하지 않은 변경사항때문에 재배포를 해야할 수 도 있습니다.
2. 도메인은 변화에 유연하게 대처해야합니다. 만약에 도메인이 그대로 외부로 나가게 됐는데, 해당 도메인의 전체를 갈아 엎어야하는경우가 생기게 됐는데, 사용자가 버전업데이트를 하지 않으면, 해당 버전을 사용하는 사용자를 위해서 변경해야할 도메인은 변경하지 못합니다. (변경해버리면, 해당 사용자는 버그가 나겠죠?, 이 때문에 레거시가 계속 남게됩니다.)
3. 의존성을 잘 관리하는 이유는 각 영역간의 영향성을 최소화 하기 위함입니다. view는 사용자의 영역이고, domain 은 가장 중요한 비지니스 영역인데 이 둘이 의존성이 엮어서 서로에게 영향을 줘야할 이유가 없습니다!
4. 아래 이유

이동욱님 블로그 글을 참고로 달아둘게요.

여기서 Entity 클래스와 거의 유사한 형태임에도 DTO 클래스를 추가로 생성했는데요.
절대로 테이블과 매핑되는 Entity 클래스를 Request/ Response 클래스로 사용해서는 안됩니다.
Entity 클래스는 가장 Core한 클래스라고 보시면 되는데요.
수많은 서비스 클래스나 비지니스 로직들이 Entity 클래스를 기준으로 동작합니다.
Entity 클래스가 변경되면 여러 클래스에 영향을 끼치게 되는 반면 Request와 Response용 DTO는 View를 위한 클래스라 정말 자주 변경이 필요합니다.

이런 이유들은 지금같은 콘솔프로그램같은 작은 서비스에선 와닿지 않을거에요!.. 이해가 안되더라도 경험하시면서 이해하도 좋을 것 같아요..

[joseph415]

해당 리뷰들에 대한 질문은 여기 계속 달아주시면 확인하면서 답글 달아드리겠습니다.