Skip to content

[로또 게임] 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

Open
wants to merge 4 commits into
base: eoeh04
Choose a base branch
from

Conversation

eunii
Copy link

@eunii eunii commented Jul 21, 2021

EnumMap 과 초기화

-참고링크

list 여러 개 합치는 함수

commit 메시지 컨벤션

SonarLint사용법

@eunii eunii changed the title [자동차 경주 게임] Step 2 리뷰 요청드립니다 [로또 게임] Step 2 리뷰 요청드립니다 Jul 21, 2021
Copy link

@Deocksoo Deocksoo left a 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()) {

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> {

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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LottoMachine이 너무 많은 역할을 담당하고 있는 것 같아요.
기능과 역할에 맞게 객체를 조금 더 분리해보는 것은 어떤가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants