[로또] 효오 미션 제출합니다. #29
Conversation
…ning lotto numbers method
…on classes and interfaces
…al lotto, autto lotto
| this(createRandomNumber()); | ||
| } | ||
|
|
||
| public LottoNumber(int number) { |
|
|
||
| import java.util.*; | ||
|
|
||
| public abstract class Lotto { |
There was a problem hiding this comment.
추상 메소드도 없는데 굳이 추상 클래스로 구현한 이유가 있는가?
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class ManualLotto extends Lotto { |
There was a problem hiding this comment.
굳이 Lotto를 상속하지 말고 Lotto를 ManualLotto처럼 사용하면 안될까?
There was a problem hiding this comment.
피드백 주신 내용을 토대로 Lotto를 ManulLotto 처럼 사용했더니 AutoLotto가 훨씬 더 간결해졌습니다!
|
|
||
| import java.util.List; | ||
|
|
||
| public class WinningLotto extends Lotto { |
There was a problem hiding this comment.
상속보다는 조합(composition) 관계를 맺는 것은 어떨까?
즉, WinningLotto의 인스턴스 변수로 Lotto를 사용하는 것은 어떨까?
| @@ -0,0 +1,34 @@ | |||
| package lotto.domain.rank; | |||
|
|
|||
| public class FirstRank extends AbstractRank { | |||
There was a problem hiding this comment.
FirstRank ~ FifthRank까지 중복이 너무 많다.
클래스 하나만 추가한 후 인스턴스를 통해 1등 ~ 5등을 구분해도 되지 않을까?
There was a problem hiding this comment.
공통된 부분을 부모 클래스에서 구현했는데 부모 클래스 인스턴스 변수의 접근 제어자를 protected로 두어도 괜찮은지 궁금합니다...
| @@ -0,0 +1,7 @@ | |||
| package lotto.domain.rank; | |||
|
|
|||
| public interface RankImpl { | |||
There was a problem hiding this comment.
일반적으로 Impl은 구현체에서 사용하는데 인터페이스에서 사용하는 것은 적절하지 않아 보임
There was a problem hiding this comment.
피드백 주신 내용 반영했습니다! 제가 잘못 알고 있었네요...
| @Test | ||
| void 로또_숫자_생성하는_테스트() { | ||
| LottoNumber lottoNumber = new LottoNumber(45); | ||
| assertThat(lottoNumber).isEqualTo(new LottoNumber(45)); |
There was a problem hiding this comment.
두 개의 LottoNumber 인스턴스가 값과 더불어 메모리 주소까지 같은지 검증하는 테스트도 추가해 본다.
| void 로또_상태_2등_알아내기_테스트() { | ||
| WinningLotto winningLotto = new WinningLotto(Arrays.asList(1, 2, 3, 4, 5, 7), 6); | ||
| Rank rank = lottos.matchLottoRank(winningLotto); | ||
| System.out.println(rank); |
There was a problem hiding this comment.
출력해서 확인하지 말고 assertThat()을 통해 테스트 결과를 검증해본다.
| } | ||
|
|
||
| private static boolean isOverlap(List<LottoNumber> lottoNumbers) { | ||
| Set<LottoNumber> checkLottoNumbers = new HashSet<>(lottoNumbers); |
There was a problem hiding this comment.
Set으로 중복을 검증한다면 이 클래스의 List<LottoNumber> 대신 Set<LottoNumber>를 사용하는 것은 어떨까?
There was a problem hiding this comment.
어떠한 행동을 할지에 대해 중점으로 생각하고 접근 했어야 했는데
데이터 중심으로 생각하고 인스턴스 변수를 미리 정해서 메서드를 만들다 보니 적절한 컬렉션을 사용하지 못했던 것 같았습니다.
피드백 주신 내용에 대해 많이 생각해보고 반영했습니다!
There was a problem hiding this comment.
피드백 반영 잘 했네요. 💯
이전보다 많이 개선된 느낌이 드네요.
로또 미션의 경우 merge된 후 다음 단계를 진행하려면 woowacouse에 merge된 코드에서 다시 시작해야 충돌이 발생하지 않을 겁니다. 다음과 같은 과정을 진행한 후 다음 단계 진행할 것을 추천해요.
"git remote add upstream woowacourse_저장소" -> "git fetch upstream 자신의 github_id" -> "git reset --hard upstream/자신의 github_id"
No description provided.