-
Notifications
You must be signed in to change notification settings - Fork 8
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
[로또 게임] 권용현 과제 제출합니다. #3
base: main
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.
추상화 관점에서 리뷰했습니다.
추상화가 잘되면 자동으로 테스트 짜기 편한 코드가 됩니다.
수정 후에 유닛 테스트도 추가해보세요~
src/main/java/lotto/Game.java
Outdated
Game(RandomNumber randomNumber, Input input, Print print) { | ||
this.randomNumber = randomNumber; | ||
this.input = input; | ||
this.print = print; |
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.
외부에서 주입받은거는 좋은 시도입니다.
이걸 Interface로 추상화 시켜서 의존성을 낮추는 방식으로 리팩토링해보세요
src/main/java/lotto/Game.java
Outdated
return list; | ||
} | ||
|
||
private void calculateWinCounts(int[] place, int idx, List<int[]> winCounts) { |
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.
프로그래밍에도 함수의 멱등성이라는 개념이 있는데요.
검색해서 찾아보고 멱등성을 만족하도록 함수를 리팩토링해보세요
src/main/java/lotto/Application.java
Outdated
public class Application { | ||
public static void main(String[] args) { | ||
// TODO: 프로그램 구현 | ||
Game game = new GameImpl(RandomNumberImpl.getInstance(), InputImpl.getInstance(), PrintImpl.getInstance()); |
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.
잘했습니다 👍
이렇게 인터페이스화 시키면 테스트 코드 짜기가 편해질거에요
if (randomNumber == null) { | ||
randomNumber = new RandomNumberImpl(); | ||
} | ||
return randomNumber; |
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.
굳이 싱글톤으로 구현안해도되는 상황이긴한데 이왕했으니 제대로 구현해보면 좋을 것 같네요.
싱글톤에는 여러 구현방식이 있는데 보통 LazyHolder 방식으로 많이 구현하니까 검색해서 찾아보고 리팩토링해보세요~
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.
싱글톤 구현 방법들에 대해서 내일 이야기 해볼건데 한 번 공부해보세요 :) 지난 시간에 한번 언급했던 것 같네요.
변수명 짓는것이 아직 미숙한것 같습니다. 계속해서 리팩토링 해 나가도록 하겠습니다.