Conversation
- 특정 방에 문제로 제공될 퀴즈 리스트라는 의미를 가지는 클래스 이름으로 명확하게 변경
- 특정 방에 문제로 제공될 퀴즈 리스트라는 의미를 가지는 클래스 이름으로 명확하게 변경
- 이름을 통일화하고, 조금 더 도메인에 맞게 이름 수정 - GamePlayEngine 은 presentation 단에서 요청을 받도록 패키지 이동
Jeongjjuna
added
🔥 Bug
| @@ -1,9 +1,9 @@ | |||
| package yjh.cstar.play.domain.quiz | |||
|
|
|||
| class Quizzes(private val quizzes: List<Quiz>) { | |||
| class RoomQuizSet(private val quizzes: List<Quiz>) { | |||
There was a problem hiding this comment.
확실히 클래스 이름만 보고도 어떤 기능을 하는지 바로 이해할 수 있네요!👍👍
There was a problem hiding this comment.
더 좋은 네이밍이있다면 추천해주세요!
There was a problem hiding this comment.
리팩토링 계속하면서 더 좋은 네이밍이 있다면 pr 올리겠습니다!
| @@ -15,7 +15,7 @@ class GameService( | |||
| val memberService: MemberService, | |||
| val roomService: RoomService, | |||
| val quizService: QuizService, | |||
| val gamePlayService: GamePlayService, | |||
| val gamePlayEngine: GamePlayEngine, | |||
There was a problem hiding this comment.
제가 이해한 바로는, 다른 패키지와의 통일성을 위해 컨트롤러에서 서비스 단에 요청을 전달하여 게임 진행 로직을 처리하는 방식으로 변경된 것 같습니다. 여기서 특정 패키지의 서비스 단에서 다른 패키지의 컨트롤러 단을 주입받아 사용하는 경우, 계층 간의 방향성이 깨지거나 사이드 이펙트가 발생할 가능성은 없을까요? (계층 간의 방향성은, 컨트롤러 -> 서비스 -> 리포지토리로 이어지는 계층 구조를 의미합니다.)
제가 잘 모르는 부분이라 설명해주시면 큰 도움이 될 것 같습니다. 👍👍
There was a problem hiding this comment.
우선 game 패키지와 play 패키지를 서로 다른 두개의 서버라고 가정했습니다.
예를들어 game 서비스는 게임 시작 api 요청을 처리하고, 게임 실행 로직에 대한 이벤트를 외부 서버에 발급한다는 느낌이었습니다.
그래서 이 경우에는 game서버에서 play서버라는 외붕 서버에 요청하는것처럼 구현하였고, 실제 외부 서버의 역할을 하는 play패키는 컨트롤러에서 api요청을 받는방식과 같다고 생각해서 presentation 패키지에 두었습니다.
(물론 현재는 단일 서버이긴 하지만요..!)
There was a problem hiding this comment.
네네 이해했습니다. 단일 서버이지만 확장성을 고려해서 두 패키지를 서로 다른 서버라고 가정한거다! ㅎㅎ 이해했습니다. 그 부분을 우리 리드미나 프로젝트 소개할 때 적는 것도 좋을 것 같습니다. 😁😁
There was a problem hiding this comment.
추후에 멀티모듈로 분리해서 서버 분리 실습해봐도 재밌을 것 같아요!(학습용..)
| data class GameConfig( | ||
| val answerProvider: AnswerProvider, | ||
| val gameNotifier: GameNotifier, | ||
| val rankingHandler: RankingHandler, | ||
| val gameResultService: GameResultService, | ||
| val roomService: RoomService | ||
| ) |
There was a problem hiding this comment.
혹시 GameConfig는 어떤 역할을 하는지 여쭤봐도 될까요?🤗
There was a problem hiding this comment.
GameConfig는 실제 QuizGame 클래스가 외부 infrastructure의 기능을 사용하기우해 주입받아야하는 Bean 들입니다.
위의 예시를보면, 5개 모두를 GameQuiz의 생성자로 주입을 받는다면 파라미터가 너무 많아질 것 이라고 생각해서 별도의 GameConfig에 사용할 외부 주입 객체들을 담아놓고 파라미터로 GameConfig를 받는식으로 구현하였습니다.
There was a problem hiding this comment.
오... 이런식으로 data class를 활용할 수도 있겠네요😍😍 다만 클래스명에서 살짝 혼동이 왔던 거 같아요. 더 좋은 네이밍이 어떤게 있을지 저도 계속 생각해보겠습니다. 혹은 해당 명칭이 관례적으로 쓰이는 부분일까요?
There was a problem hiding this comment.
아 관례적으로 쓰이는것은 아니었는데, Game에 필요한 설정파일(외부인프라스트럭쳐클래스)이라고 해서 Config라고 붙였습니다.. 좋은 이름 생각해볼게요!
| @@ -7,7 +7,7 @@ interface RankingHandler { | |||
|
|
|||
| fun initRankingBoard(roomId: Long, players: Players) | |||
|
|
|||
| fun assignScoreToPlayer(roomId: Long, playerId: Long) | |||
| fun assignScoreToRoundWinner(roomId: Long, roundWinner: Long) | |||
There was a problem hiding this comment.
의도를 명확히 전달하기위해 조금 과장해서 적은것도 있고, 실제로 조금 부자연스러워 보이는 것도 있는 것 같습니다.
그래도 이렇게 네이밍 연습하다보면 나중에는 자연스러워질 것 같아요!
There was a problem hiding this comment.
의도를 명확히 하기 위해 조금 부자연스럽더라도 길게 적는 게 더 좋아보입니다. 다른 사람의 코드를 읽는 개발자 입장에서 그렇게 하는게 이해하기 더 쉬운 것 같아요!
There was a problem hiding this comment.
넵넵 팀원들이 이해된다면 그걸로 된 것 같아요
관례나 관용 표현 있다면 추천해주세요!!
| validateFinished() | ||
|
|
||
| val currQuiz = quizzes[currQuizNo++] | ||
| return Pair(currQuizNo, currQuiz) |
There was a problem hiding this comment.
(currQuizNo, currQuiz) 두 개가 쌍으로 반환이 되네요! 😄 저도 사용해봐야겠습니다!
There was a problem hiding this comment.
튜플로 보기 좋을것이라 생각했는데...! 찾아보니 단점이 존재했네요...
이펙티브 코틀린 읽으면서 발견하였는데요, Pair 나 Triple 보다는 data class를 적극적으로 사용하길 권장한다고 합니다.
그 이유는 Pair 안에서 값을 꺼낼때 의미가 명확히 전달되지 않을 수 있기 때문네요.(그 외에 data class 의 장점들..)
우선 data class를 사용하도록 변경해보겠습니다!!
| @@ -39,7 +39,6 @@ class Room( | |||
|
|
|||
| fun endGameAndResetRoom() { | |||
| resetStatusIfInProgress() | |||
| currCapacity = 0 | |||
There was a problem hiding this comment.
해당 부분을 삭제하신 이유가, 방 상태를 초기화하면 방 인원 수를 초기화하는 것은 의미가 없기 때문일까요?🤔🤗
There was a problem hiding this comment.
해당부분은 원래 게임이 끝났을 때 방에 참여한 인원수를 0으로 바꾸는 방식이었습니다.
하지만 실제 프론트엔드 영역에서 게임이 끝나도 해당 방에 머무르도록 구현이 되어있습니다.
그렇기 때문에 한 게임이 끝나도라도 플레이어는 해당 방에 계속 존재할 것이고, 계속해서 게임을 시작할 수 있는 상태입니다.
방 인원수를 초기화하지 않아도 되는 상황이었습니다!
There was a problem hiding this comment.
그럼 해당 방에서 게임이 끝난 후 게임을 계속 진행할지 또는 방을 나갈지 선택하는 api를 추가로 만들어야 할 것 같습니다. 다음 회의때 논의 해봐요😆
There was a problem hiding this comment.
아 게임이 끝나면 그냥 방에 참여한생태로 유지가 되어서, 그냥 한번 더 플레이하고 싶다면 또 시작을 누르면 돼요!
새로운 api가 없어도 됩니다!
- 튜플에서 데이터를 꺼낼때 의미전달이 모호해질 수 있다. (이펙티브 코틀린 참고)
| data class QuizInfo( | ||
| val quizNo: Int, | ||
| val quizId: Long, | ||
| val quiz: Quiz, | ||
| ) |
There was a problem hiding this comment.
객체를 생성해야할 때 먼저 data class로 가능한지 생각해봐야겠습니다! 관련해서 리뷰 달려고 했는데 바로 반영해주셨네요! 역시 지훈님!! 입니다.😁😁
#️⃣ 관련 이슈
📝 작업한 내용
💬 논의하고 싶은 내용