-
Notifications
You must be signed in to change notification settings - Fork 49
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
[로또 미션] 김의천 미션 제출합니다. #71
Open
wzrabbit
wants to merge
21
commits into
next-step:wzrabbit
Choose a base branch
from
wzrabbit:main
base: wzrabbit
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 발행된 로또 값들을 그대로 로또 클래스에 생성자로 넣기 때문에, 모킹이 필요하지 않으며, 상속 또한 필요하지 않음
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 리뷰어님 👋🏻 로또 미션을 제출합니다. 잘 부탁드립니다!
기능 자체는 많이 들어있지 않은 것 같은데 엄밀하게 코드를 짜니 어마무시하게 거대한 코드가 되어버리는 것 같군요
이번 미션에서 신경써본 점 ✅
for
문의 사용을 줄이고,stream
,forEach
등을 사용해 코드의 의도를 나타내었습니다.stream
이 장난아니게 복잡하더군요.일급 컬렉션 사용 소감
상당히 좋았어요!
미션 진행 전 구글링을 하면서 클래스들을 개발자분들이 어떻게 다루나 보았는데 신기하게도 도메인 로직을 담당하는 클래스 내부 자체에 검증 로직을 많이들 두었더라고요. 그 때는 그런갑다 하고 넘어갔는데 이렇게 직접 검증 로직들을 그 클래스에 넣고 관리해주니 코드의 신뢰도가 많이 올라갔다는 것을 느꼈습니다.
결론적으로 데이터를 관리하기에 안전한 방식처럼 느껴졌어요. 특히 여러 클래스들을 조합하여 새로운 클래스를 만들거나 복잡하게 데이터가 오갈 때 이 장점이 부각될 것 같아요.
enum 사용 소감
음... 솔직히 아직까지는 크게 장점이 와닿지는 않았던 것 같아요. 상수들만을 관리하는 클래스를 만들어서 쓸 수도 있었을 것 같고, 심지어 그게 더 간단한 문법이었을 수도 있을 것 같아요. enum을 어떨 때 쓰면 좋을지, 상수들을 관리하는 클래스를 만들었을 때와는 무슨 차이가 있는지 좀 더 알아봐야겠네요.
~~~.getValue()
남발하다 보니 너무 길어지더라고요. 물론 의미를 나타낸다면 충분히 이름은 길어질 수 있지만 이건 좀 다른 느낌인 것 같아요.getValue()
를 뺀다고 의미가 퇴색되는 것도 아닐테고요.이번 미션에서는 크게 헤맨 점은 없지만, 번거로운 점은 좀 많았던 것 같아요. 자바스크립트에서는 쉽게 했던 배열 합치기, 배열 그대로 대입하기, 맵핑하기가 자바에서는 상당히 불편하게 느껴졌어요. 아마 코드 곳곳에 복잡한 부분이 있을 거에요. 자바스크립트의 문법이 그리워지네요. 😢
아래는 실행시켜 본 화면이에요.
그럼, 잘 부탁드리겠습니다!