[유안] 로또 2단계 미션 제출합니다 #221
Merged
[유안] 로또 2단계 미션 제출합니다 #221
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
added 8 commits
February 26, 2020 13:05
List 대신 Map 사용하면서 of 메서드 성능 개선 static 값들 관리를 내부에 inner 클래스가 하는 대신 repository class를 따로 만들어서 명시성 향상 최대, 최소 정보를 public으로 변경함으로서 외부에서 로또 번호 객체를 다루기 좀 더 용이하게 변경
티켓 객체와 당첨번호 객체의 공통 부분을 상위 클래스로 분리, 상속처리 원래 dto는 하나만 있었는데 티켓dto와 당첨번호dto 분리
1. Dto 삭제
프로그램의 주요 동작보다 dto 관련 동작에 더 신경을 쓰게 되는 것을 느꼈다.
당장 더 중요한 것에 집중하기 위해 삭제했다.
2. WinningNumbers 객체 안에서 LottoTicket 객체를 소유하도록 변경
LottoTicket 객체는 일급 컬렉션의 역할을 한다.
WinningNumbers가 소유하고 있는 LottoTicket은 일등 티켓을 의미한다.
LottoStore 객체는 static 메서드만을 가지며 LottoStore 객체를 생성하지 않아도 작동할 수 있도록 변경 LottoStore의 메서드는 BuyingStrategy 인터페이스를 인자로 받아 여러가지 방법으로 표를 생성할 수 있도록 변경
기존의 BuyingStrategy 인터페이스를 상속해서 ManualBuyingStrategy 구현
View 약간 추가 기존에 만들었던 객체들 조합하여 적절한 동작하도록 구현
사용하지 않는 import 삭제 띄어쓰기 등 컨벤션 안지켜져있던 부분 수정
jeonghoon1107
suggested changes
Mar 3, 2020
|
|
||
| public List<LottoTicket> generateTickets(T t); | ||
| public static List<LottoTicket> generateTickets(BuyingStrategy strategy, Object buyingInformation) { | ||
| return strategy.generateTickets(buyingInformation); |
There was a problem hiding this comment.
해당 부분에서 ManualBuyingStrategy와 RandomBuyingStrategy를 이용해 List<LottoTicket>을 만들면 어떨까요?
List<BuyingStrategy> strategy = Arrays.asList(new ManualBuyingStrategy(), new RandomBuyingStrategy())
List<LottoTicket> tickets = strategy.stream().filter(accept).map(BuyingStrategy::generateTickets)~~
public interface BuyingStrategy {
boolean accept(~~);
List<LottoTicket> generateTickets(BuyingInformation information);
}
으로 사용하면 어떨까요?
added 2 commits
March 4, 2020 11:07
LottoStore 객체 삭제 ManualBuyingStrategy 에서는 수동 로또만 생성 담당하게 수정 RandomBuyingStrategy 가 더이상 ManualBuyingStrategy 안에서 호출되지 않음 BuyingStrategy 객체들과 BuyingInformation 객체와의 조합만으로 생성하도록 수정
|
말씀하신 부분에 대해서 수정했습니다. 수정하며 개선 되었다고 느낀 점들입니다.
|
jeonghoon1107
approved these changes
Mar 5, 2020
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| @DisplayName("로또 구매 테스트") | ||
| public class BuyingStrategyTest { |
There was a problem hiding this comment.
RandomBuyingStrategy, ManualBuyingStrategy 두 경우를 합친 테스트도 있으면 좋을 것 같아요.
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.
늦어서 죄송합니다.