조찬우 - 숫자야구 게임 #2
Conversation
There was a problem hiding this comment.
찬우님! 미션하시느라 고생하셨어요~
일단 찬우님이 PR에 남기신 부분에 대한 답변을 드리자면
랜덤 숫자 생성, 생성 값이 세자리 수로 정해져 있는 만큼 컬렉션이 아닌 일반 배열을 사용해 구현하고자 했으나 중복제거의 이점을 가져가기 위해 Set 컬렉션과 List 를 활용해 구현. (0 포함 되는 문제로 인식해서 발생한 문제)
왜 세자리 수라서 배열로 생성을 하려고 하셨는지에 대한 의문이 들어요. 중복 제거를 위해서 Set을 잘 활용하셨습니다
Input, Output 클래스 생각나는 대로 개발은 했으나 테스트에 있어서 어려움이 있을 것 같은 클래스. (실제로 테스트 아직 못함) 개별 메서드들이 따로 노는 느낌. 리펙토링 예정
Input과 Output은 일단 테스트 진행하지 않으셔도 됩니다!
그리고, 미션을 미리 시작하시고 본인이 하실 수 있는 선에서 충분히 리팩토링을 하시고 제출하시는게 좋습니다 ㅎㅎ
game 패키지와 더불어 구현된 클래스들이 대체적으로 객체지향적으로 짜여지지 않은것 같아 맘에 안듦. 상호 객체 간의 의존성 관계가 매우 불안정해 보임.
어떤 부분이 객체지향적이지 않다고 생각하셨나요? 일단, 객체지향은 다음 미션에서 더 얘기하면 좋을 것 같아요! Clean Code와 Unit Test에 집중하셔도 될 것 같습니다!
입력 값에 대한 검증도 이 클래스에서 이루어진다
중복 없는 세자리 랜덤 숫자 생성 테스트
Constant 클래스 improt
코드 대폭 수정처음에 기능 구현만 하자는 느낌으로 코드를 짜서 난잡했던 코드들을 모두 갈아 엎었습니다.
살짝 고민이 되었던 부분 몇가지 제 코드에 코멘트 달아놓겠습니다~! |
| public class OutputConsoles implements OutputConsole { | ||
| } |
There was a problem hiding this comment.
여 타 애플리케이션 개발할 때 써 본적 없는 방식이긴 한데
타입 추론을 위한 껍데기 클래스로 만들어봤습니다.
어떻게 생각하시는지 여쭤보고 싶네요!
|
|
||
| import java.util.List; | ||
|
|
||
| public class BaseballGameService { |
There was a problem hiding this comment.
Service라는 네이밍을 하셨네요. 찬우님이 생각하시는 Service란 무엇인가요?
There was a problem hiding this comment.
숫자야구 게임이라는 하나의 시스템에 대한 비즈니스 로직을 수행하는 클래스라고 생각해 Service 라는 네이밍을 사용했습니다.
그런 의미에서 위의 질문과 더불어 숫자야구 게임을 시작하고 실행시키는 비즈니스 로직, 즉 도메인 로직을 담았다는 의미에서 domain 패키지로 분리했습니다. 또한 다른 비즈니스 로직에서 사용될 클래스를 모아둔 model 패키지를 분리해 역할별로 나눠봤습니다.
Spring 프로젝트에서 사용할때 저는 보통 domain 을 DB 와 관련된 클래스들을 관리하는 패키지로 사용했습니다. 사실 제가 독학인지라 이런 명확한 기준을 잡아준 사람이 없어서 이번기회에 이런 패키지 명 같은 것도 기준을 잡고싶네요!!
| import static org.junit.jupiter.api.Assertions.*; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| class BaseballRuleCheckerTest { |
There was a problem hiding this comment.
테스트 잘 작성해주셨네요. assertAll 까지 사용하시면서요!
assertAll을 썼을 때, 어떤 장점이 있는지 알고 계신가요?
There was a problem hiding this comment.
제가 아는 것으로는 assert 가 여러개 있을 경우, 단일 assertThat 의 나열은 하나의 assert 가 실패하면 테스트가 종료되지만
assertAll 을 사용하면 모든 assert 를 검증한 후 실패한 테스트를 보여준다는 것에 장점이 있는 것으로 알고 있습니다.
이외에 다른 장점도 있을까요?!🤓
| public interface OutputConsole { | ||
|
|
||
| default void printResult() { | ||
| System.out.println("결과입니다"); | ||
| }; | ||
|
|
||
| default OutputConsole checkTypeByBallCount(BallCountDto ballCount) { | ||
| OutputConsole outputConsole = null; | ||
|
|
||
| if (ballCount.isNothing()) { | ||
| outputConsole = new OutputConsoleNothing(); | ||
| } else if (ballCount.isOnlyBall()) { | ||
| outputConsole = new OutputConsoleOnlyBall(ballCount.getBall()); | ||
| } else if (ballCount.isOnlyStrike()) { | ||
| outputConsole = new OutputConsoleOnlyStrike(ballCount.getStrike()); | ||
| } else if (ballCount.isStrikeAndBall()) { | ||
| outputConsole = new OutputConsoleStrikeAndBall(ballCount.getStrike(), ballCount.getBall()); | ||
| } | ||
|
|
||
| return outputConsole; | ||
| } | ||
| } |
There was a problem hiding this comment.
인터페이스로 view를 추상화한건 정말 좋은 선택입니다!
default 메서드를 사용했는데, default 메서드는 왜 java에 도입되게 되었을까요?
또한, default 메서드를 사용하면 어떤 문제가 발생할 수 있을까요?
또한, view를 interface로 분리한 점은 좋은데, strike개수와 ball 개수에 따라서 클래스가 나눠졌는데 조금은 투머치 한 것 같다는 생각이 들어요! 물론 시도는 너무 좋습니다.
단순하게 strikeCount와 ballCount를 받아서 1 스트라이크 2볼 이런식으로 출력만해줘도 좋을 것 같아요!
There was a problem hiding this comment.
인터페이스에 메서드를 추가 개발하고자 할 때, 이미 해당 인터페이스를 구현한 구현체가 셀 수 없이 많다면 그 모든 구현체에 대해
추가된 메서드를 구현해야한다는 문제에서 default 키워드를 도입한것으로 알고 있습니다.
검색해보니 문제는 자바의 다중 상속에서 같은 이름의 default 메서드가 있을때의 문제인 것 같네요!! 제가 놓친 다른 문제점도 있을까요~?!
저도 하면서 좀 오반가? 싶긴 했습니다 ㅋㅋㅋㅋ 리뷰해주신대로 좀 더 단순하게 가져가도 좋을 것 같네요!
| private static final int END = 9; | ||
|
|
||
| public int generate() { | ||
| return START + new Random().nextInt(END); |
There was a problem hiding this comment.
| return START + new Random().nextInt(END); | |
| return START + new Random() | |
| .nextInt(END); |
기능 구현시 아쉽다고 생각한 부분
랜덤 숫자 생성
생성 값이 세자리 수로 정해져 있는 만큼 컬렉션이 아닌 일반 배열을 사용해 구현하고자 했으나
중복제거의 이점을 가져가기 위해 Set 컬렉션과 List 를 활용해 구현. (0 포함 되는 문제로 인식해서 발생한 문제)
Input, Output 클래스
생각나는 대로 개발은 했으나 테스트에 있어서 어려움이 있을 것 같은 클래스. (실제로 테스트 아직 못함)
개별 메서드들이 따로 노는 느낌. 리펙토링 예정
game 패키지
game 패키지와 더불어 구현된 클래스들이 대체적으로 객체지향적으로 짜여지지 않은것 같아 맘에 안듦.
상호 객체 간의 의존성 관계가 매우 불안정해 보임.
위와같은 문제로 테스트에서도 어려움을 겪었습니다. 리팩토링 하고 다시 테스트 할 예정인데 코드리뷰 하면서 조언 부탁드려요😋