-
Notifications
You must be signed in to change notification settings - Fork 12
[로또 게임] Step 2 리뷰 요청드립니다 #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: eoeh04
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 다은선생님!
일급컬렉션을 적용해보려고 하셨군요 ㅎㅎ 좋은 시도인 것 같아요 👍
필요한 객체들이 점점 더 잘 드러나기 시작했군요.
다만 아직 일급컬렉션이 의미에 맞게 적절히 사용되고 있지는 않아요.
일급컬렉션은 단순히 한 가지 종류의 패턴이 아니라, 객체지향과 관련된 핵심적인 철학들을 담고 있어요.
어떤 데이터를 가져야하는가 보다는, 어떤 기능을 해야하는가에 초점을 맞추고 조금 더 고민해보시면 좋을 것 같아요.
|
||
public RankMap getRankResult(Lottos lottos, List<Integer> winningNumber, int bonusNumber) { | ||
RankMap rankMap = new RankMap(Rank.class); | ||
for (Lotto lotto : lottos.getLottos()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lottos 일급컬렉션에서 로또를 꺼내서, 각각의 Lotto로부터 Rank를 만들어 RankMap에 넣고 있군요.
일급컬렉션을 만드신 이유를 한 번 더 고민해보시면 좋을 것 같아요. java Collection을 한 번 감싸고, 다른 이름을 붙여주기만 하면 일급컬렉션으로 생각해도 좋을까요? java Collection을 감싸는 이유는 무엇인가요?
Lottos로 감싼 collection을 되도록 밖으로 꺼내지 않고 해결할 수 있는 방법은 없나요?
|
||
import java.util.EnumMap; | ||
|
||
public class RankMap extends EnumMap<Rank, Integer> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java Collection을 상속받고 있군요. RankMap은 EnumMap과 무슨 차이가 있나요?
이렇게 상속 받은 이유가 무엇인가요?
import java.util.stream.Collectors; | ||
import java.util.stream.IntStream; | ||
|
||
public class LottoMachine { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LottoMachine이 너무 많은 역할을 담당하고 있는 것 같아요.
기능과 역할에 맞게 객체를 조금 더 분리해보는 것은 어떤가요?
EnumMap 과 초기화
-참고링크
list 여러 개 합치는 함수
commit 메시지 컨벤션
SonarLint사용법