Skip to content
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

[2단계 - 로또(수동)] 후디(조동현) 미션 제출합니다. #439

Merged
merged 59 commits into from
Mar 7, 2022

Conversation

devHudi
Copy link

@devHudi devHudi commented Mar 1, 2022

안녕하세요, 또링! 즐거운 휴일 보내셨나요?

남겨주신 리뷰를 보고, 반영할 수 있는 부분은 모두 반영해보았어요. 1단계 때와 같이 질문이 있다면 코멘트로 남기겠습니다!

또한 1단계 2번째 피드백(#385) 에도 답글을 달아두었습니다. 친절한 피드백 항상 감사드립니다, 또링! 😸

@devHudi devHudi changed the base branch from main to devhudi March 1, 2022 14:43
devHudi added 26 commits March 1, 2022 23:57
@devHudi
Copy link
Author

devHudi commented Mar 3, 2022

안녕하세요 또링! 2단계 두번째 리뷰 요청입니다! 또링 피드백 덕분에 정말 많은 것에 대해 고민하고, 성장할 수 있었어요. 정말 감사드려요 😄 다른 분들의 코멘트 수를 보니 제가 유난히 좀 많은 것 같은데, 제가 질문이 좀 많죠? ㅎㅎ.. 😓

잘 이해가 안가거나 말끔하게 고민이 해결이 안된것들은 코멘트에 답글로 달았어요. 이번에도 잘 부탁드려요!

Copy link

@jnsorn jnsorn left a comment

Choose a reason for hiding this comment

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

안녕하세요, 후디! 피드백 반영해주신 것 확인했어요. :)
이미 많은 부분에 코멘트가 열려있어 추가로 리뷰는 안드렸습니다.
코멘트 답변 확인해보시고 다시 요청주세요. 거의 다 왔네요! 👏🏻

@devHudi
Copy link
Author

devHudi commented Mar 6, 2022

또링이 주신 피드백을 바탕으로 다시한번 개선해서 리뷰 요청 드립니다! 컨트롤러의 관심사와 책임 범위를 어떻게 설정하면 좋을지 많이 고민했는데, 다소 서툰 결과물이 나온 것 같아서 아쉬워요 😢 스트림에 대하여 공부를 더 해야겠다는 생각도 많이 들었습니다! 이번에도 잘 부탁드려요!

Copy link

@jnsorn jnsorn left a comment

Choose a reason for hiding this comment

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

후디 안녕하세요 :) 피드백 반영해주신 부분 확인했어요. 👍🏼
코멘트 답변과 리뷰 하나 남겼어요. 확인해보시고 반영해주세요!
마지막 리뷰인 것 같네요. 😃

src/main/java/domain/Lotto.java Outdated Show resolved Hide resolved
src/main/java/controller/LottoController.java Outdated Show resolved Hide resolved
Copy link

@jnsorn jnsorn left a comment

Choose a reason for hiding this comment

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

안녕하세요, 후디! 피드백 잘 반영해주셨네요. 👍🏼
게더에서 말씀드린대로 후디의 많은 질문과 고민들 덕분에 저도 같이 성장한 것 같아요 🌱
미션 구현하느라 고생 많으셨고, 남은 미션도 화이팅입니다!

Comment on lines +16 to +20
static {
LOTTO_NUMBERS = IntStream.rangeClosed(MINIMUM_LOTTO_NUMBER, MAXIMUM_LOTTO_NUMBER)
.mapToObj(LottoNumber::new)
.collect(Collectors.toUnmodifiableList());
}
Copy link

Choose a reason for hiding this comment

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

👍🏼

Comment on lines +28 to +33
public static LottoNumber from(int number) {
validateRange(number);

int lottoNumberIndex = number - 1;
return LOTTO_NUMBERS.get(lottoNumberIndex);
}
Copy link

Choose a reason for hiding this comment

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

조금 더 적절한 자료구조가 있을 것 같아요 🧐 key를 주면 해당 key에 매핑된 value를 꺼내주는..!!

@jnsorn jnsorn merged commit f47d983 into woowacourse:devhudi Mar 7, 2022
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