[로또 게임 1단계] 리뷰 부탁드립니다 #11
Conversation
There was a problem hiding this comment.
지우선생님! 요구사항에 맞게 잘 작성하셨어요.
일급컬렉션 활용과 MVC 모델을 적용하려는 노력과 고민 흔적이 코드에서 드러납니다.
코멘트 남겼으니 확인해보시고 한 번 개선해보세요. :)
질문에 대한 답변도 남겼습니다.
Public 상수
도메인 안에 정의한 상수의 접근제어자를 public으로 해서 다른 클래스에서 접근해도 괜찮은가요? 저희 코드에서는 LottoNumber.UPPER_BOUND, Lotto.PRICE 등과 같은 상수를 여러 군데에서 활용했습니다. 활용도와 가독성이 좋아서 쓰면서 만족스러웠는데, 지양해야 하는 것인지 궁금합니다.
전체 어플리케이션에서 빈번하고 유용하게 활용되는 중요한 상수라면 public으로 사용해도 문제 없습니다. 여기서 상수는 지금처럼 primitive type의 값일 수도 있지만, 객체일 수도 있습니다. 다만, 공개된 데이터가 많아질수록 참조되는 횟수가 자연스레 많아지고 이는 객체간 결합도를 높이는 방향으로 이어지게되니 객체지향적 ㅡ역할과 책임으로 명확하게 구분되어야하는ㅡ 으로 설계하고 있는가 꾸준히 스스로 질문하며 개발해야합니다. 다른 방식으로 우회가능한 불필요한 참조는 아닌지, 객체로 포장하여 정보를 은닉한 채 활용할 수는 없는 지 등을 스스로 점검해보면 좋겠네요.
private 변수와 getter
getter 사용은 View에서의 출력이나 DTO의 멤버변수 등이 아니라면 최대한 지양해야 하는 것으로 알고 있어서, 도메인의 공개된 메서드에 최대한 로직을 담으려고 노력했습니다. 하지만 도메인 멤버변수의 접근제어자를 private으로 설정하더라도, 테스트와 로직 구현을 위해서 getter를 정의하고 사용하는 경우가 많았습니다. 이를 개선할 수 있는 방법에 대해 조언을 부탁드립니다.
흔히 TDA라 불리는 원칙을 잘 지키면 객체 설계에 있어 단순한 인터페이스를 만들어낼 수 있습니다. 객체간 커뮤니케이션 단순화는 프로그래밍 복잡도를 낮추는 가장 핵심이라 할 수 있습니다. 다만 말씀하신 것처럼, 테스트를 작성하다보면 어떤 변화를 가하고서 이에 대해 내부 상태를 꺼내야만 검증이 가능한 경우를 만나곤합니다. 이번 과제에서는 자료 구조를 일급 컬렉션으로 포장하며 getter가 필요했을 수 있겠네요. 저는 위 TDA를 의식하며 개발한다면 실용적인 관점에서 getter 정도는 편하게 열어 사용해도 된다 생각합니다. 그게 아니라면 getter 역할을 대체하는(다른 이름이지만 같은 역할을 하는) 유사 getter 메서드가 생기기 마련인데 이는 다소 억지스러운 방식으로 보입니다.
Builder 패턴과 new 생성자
저는 같은 타입의 생성자 파라미터 여러 개가 있을 때 버그를 방지하기 위해 빌더 패턴을 사용하는 것이 바람직하다고 알고 있습니다. 그렇다면 파라미터가 하나일 때나, 여러 파라미터의 타입이 다 다를 때는 new 생성자로 인스턴스를 생성하는 게 더 좋은가요? 어디선 빌더를 쓰고 또 다른 데선 new를 쓰면 코드를 봤을 때 통일성이 떨어진다는 느낌이 들어서 고민이 됩니다.
아주 멋진 고민입니다. 👍
답이 없는 질문이기는 합니다만, 제 의견을 말씀드리자면 내가 직접 문제 상황에 직면하기 전까지 시중에 유통되는 솔루션을 유예해보는 것도 괜찮더라 입니다.
튜닝의 끝은 순정이라는 말이있는데요. 저는 언어가 가진 문법이 가장 기본적인 틀이라 생각합니다.
언어에서 지원해주는 문법만으로 해결할 수 있는 문제라면(질문 내용에 담겨있는 상황처럼 파라미터가 단순하거나 구분이 확실하여 빌더를 사용하지 않아도 실수하지 않을 때) 그렇게 하는 것이 가장 좋다 생각합니다.
지우선생님께서 고민하고 질문하신 것과 반대로, 어떤 패턴으로 문제를 푸는 것이 좋더라는 것만 듣고 직접 문제 상황에서 깊이 고민해보지 않은 채 이를 수용하였을 때는 타성에 젖은 개발로 이어질 가능성이 높습니다. 근거가 약한 개발이 지속되면 당연히 본인이 작성하는 코드에 대한 자신감도 떨어져 이렇게 개발하는게 맞나 싶은 생각을 떨쳐낼 수 없겠지요.
그래서 저는 어떤 패턴이 준언어급으로 고착화되지 않기를 바랍니다.
문제 상황을 만나 깊이 고민해 볼 기회를 빼앗기기 때문입니다.
(물론 고민 끝에는 문제풀이에 아주 적극적으로 활용하여야 합니다.)
| # java-lotto 게임 | ||
| # java-Lotto 게임 | ||
|
|
||
| # 구현 기능 목록 |
| @@ -10,6 +10,13 @@ repositories { | |||
| dependencies { | |||
| testImplementation('org.junit.jupiter:junit-jupiter:5.6.0') | |||
| testImplementation('org.assertj:assertj-core:3.15.0') | |||
|
|
|||
| compileOnly 'org.projectlombok:lombok:1.18.20' | |||
There was a problem hiding this comment.
넵! getter와 빌더 패턴을 사용하려고 활용했습니다
| this.lottoService = new LottoService(); | ||
| } | ||
|
|
||
| public void start() throws IllegalArgumentException { |
There was a problem hiding this comment.
MVC 패턴을 이용해 문제를 풀려고 시도하셨네요!
다만 Model - View - Controller 역할 분리가 명확하게 되어있지 않아
현재 Controller가 View를 직접 활용하는 방식으로 코드 구현이 되어있는데요.
각자의 역할과 책임은 무엇이고, 이를 어떻게 독립적으로 분리해낼 수 있을 지, 또 왜 그러하여야 하는지 고민해보시면 좋겠습니다. 현재는 LottoController.start() 는 LottoApplication의 main과 같은 역할로 보이네요.
There was a problem hiding this comment.
Controller가 View를 직접 활용하는 방식이 왜 바람직하지 않은지 이해가 가지 않습니다. MVC 패턴에서 Controller는 View와 Model(Service) 사이의 연결 역할을 해주기 때문에 둘 다 참조하고 있어야 하지 않나요?
만약 이게 웹 어플리케이션이라면 Controller에서 request를 받아서 처리한 후 model이나 view를 만들어 리턴해줄텐데, 그 view를 만든다는 것과 view 객체의 메서드를 호출하는 것의 차이점이 뭔지 모르겠습니다.
또한, LottoApplication.main()과 LottoController.start()의 역할을 분리하기 위해 LottoController가 View에 의존하지 않도록 수정했습니다. 대신, LottoApplication이 View와 Controller를 가지고 그 흐름을 관리하는 것으로 수정했습니다. 하지만 이렇게 하면 Controller가 하는 일은 input 데이터를 받아서 output 데이터를 넘겨주는 일 뿐이잖아요? 그러면 mvc 패턴에서의 Controller 역할이 아니라, Service 도메인 역할이 되지 않나 싶습니다. 실제로 제가 수정한 코드에서 Controller가 하는 일은 Service의 메서드를 호출하는 일 뿐이니까요..!
| @Getter | ||
| @Builder | ||
| @RequiredArgsConstructor | ||
| public class PurchasePriceInputDTO { |
There was a problem hiding this comment.
네이밍을 통해 해당 클래스가 DTO라는 것이 잘 드러나게 한다면 DTO라는 명칭을 postfix로 붙이지 않아도 됩니다.
| @Getter | ||
| @RequiredArgsConstructor | ||
| @Builder | ||
| public class PurchaseResultDTO { |
There was a problem hiding this comment.
네이밍을 통해 해당 클래스가 DTO라는 것이 잘 드러나게 한다면 DTO라는 명칭을 postfix로 붙이지 않아도 됩니다.
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class PrizeTest { |
There was a problem hiding this comment.
격리된 테스트로 핵심 비즈니스 로직 테스트 수행 💯
There was a problem hiding this comment.
ParameterizedTest로 리팩토링을 해보세요~
|
|
||
| @Test | ||
| @DisplayName("개수를 입력받으면 개수만큼 랜덤 로또를 생성한다") | ||
| void testGenerateLottoSet() throws Exception { |
There was a problem hiding this comment.
불필요 throws Exception 제거
There was a problem hiding this comment.
옙 throws Exception 필요한 곳에만 남기고 제거했습니다
| import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; | ||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
|
||
| public class PurchaseCountTest { |
There was a problem hiding this comment.
불필요한 throws Exception 제거
|
|
||
| import lotto.domain.dto.WinningLottoInputDTO; | ||
|
|
||
| public class TestWinningLotto extends WinningLotto { |
|
|
||
| import java.util.HashSet; | ||
|
|
||
| public class TestLottoSet extends LottoSet { |
학습로그
Enum
일급 콜렉션
질문
Public 상수
도메인 안에 정의한 상수의 접근제어자를
public으로 해서 다른 클래스에서 접근해도 괜찮은가요? 저희 코드에서는LottoNumber.UPPER_BOUND,Lotto.PRICE등과 같은 상수를 여러 군데에서 활용했습니다. 활용도와 가독성이 좋아서 쓰면서 만족스러웠는데, 지양해야 하는 것인지 궁금합니다.private 변수와 getter
getter사용은View에서의 출력이나DTO의 멤버변수 등이 아니라면 최대한 지양해야 하는 것으로 알고 있어서, 도메인의 공개된 메서드에 최대한 로직을 담으려고 노력했습니다. 하지만 도메인 멤버변수의 접근제어자를private으로 설정하더라도, 테스트와 로직 구현을 위해서getter를 정의하고 사용하는 경우가 많았습니다. 이를 개선할 수 있는 방법에 대해 조언을 부탁드립니다.Builder 패턴과 new 생성자
저는 같은 타입의 생성자 파라미터 여러 개가 있을 때 버그를 방지하기 위해 빌더 패턴을 사용하는 것이 바람직하다고 알고 있습니다. 그렇다면 파라미터가 하나일 때나, 여러 파라미터의 타입이 다 다를 때는
new생성자로 인스턴스를 생성하는 게 더 좋은가요? 어디선 빌더를 쓰고 또 다른 데선new를 쓰면 코드를 봤을 때 통일성이 떨어진다는 느낌이 들어서 고민이 됩니다.