[2주차]객체지향 코드 연습(Jungsukwoo) #27
Conversation
Jungsukwoo
requested review from
Hoya324,
KoSeonJe and
TaetaetaE01
as code owners
There was a problem hiding this comment.
열심히 하시고 계시군요! 시간이 부족했는데, 코드를 잘 작성해주셨습니다.
의존성 주입을 외부에서 하는 것, 인터페이스를 정의한 것이 정말 좋은 것 같습니다!
말씀드리고 싶은 점은, 코드 퀄리티도 중요하지만, 완성도 있는 코드가 가장 우선순위라고 생각합니다.
좋은 퀄리티를 가진 코드도 실행이 안된다면, 쓸모 없기 때문입니다!
그리고 처음부터 좋은 퀄리티의 코드를 작성하는 것은 어렵기 때문에, 완성도 있는 코드를 작성한 뒤 리팩토링을 해가는 것이 일반적인 방법입니다.
제가 하고 싶은 말은 처음부터 100% 좋은 퀄리티로 코드를 작성하려 하지 않아도 된다는 것 입니다
고생하셨습니다!
| private final LottoService lottoService; | ||
| private final InputView inputView; | ||
| private final OutputView outputView; | ||
|
|
||
| public LottoController(LottoService lottoService, InputView inputView, OutputView outputView) { | ||
| this.lottoService = lottoService; | ||
| this.inputView = inputView; | ||
| this.outputView = outputView; | ||
| } |
There was a problem hiding this comment.
오 생성자 주입을 사용하셨네요! 좋은 선택인 것 같습니다. 그래도 이유를 알면 좋으니 의존성 주입에는 다양한 방법이 있는데 생성자를 통해 의존성을 주입하신 이유에 대해 설명해주실 수 있을까요??
There was a problem hiding this comment.
AppConfig를 통해 의존성을 관리하고 있는 것이 여러 의존 관계를 지닐 로또 미션에서 굉장히 좋은 선택이라는 생각이 듭니다.!
다만 아직까진 AppConfig은 그럼 어디서 생성해주고 관리해야할까요? 이것도 한번 고민해보시면 좋을것 같습니다!
|
여러 구조면에서 고민한 흔적이 많이 보여서 좋아보입니다! 이 기세로 완성도까지 높혀서 마무리 잘 하셨으면 좋겠습니다 |
…기 때문에 당첨 로또번호를 체크하고 결과값으로 출력하는 메소드로 변경
|
|
||
| public LottoServiceImpl(LottoRepository lottoRepository) { | ||
| this.lottoRepository = lottoRepository; | ||
| this.lottoResult = new LottoResult(); |
There was a problem hiding this comment.
LottoResult만 Service 단에서 생성해주신 이유가 있을까요?
src/main/java/lotto/model/Lotto.java
Outdated
| private void validate(List<Integer> numbers) { | ||
| if (numbers.size() != 6) { | ||
| OutputView.printErrorMessage(); | ||
| throw new IllegalArgumentException(); | ||
| } | ||
| for (Integer number : numbers) { | ||
| if (number < 1 || number > 45) { | ||
| OutputView.printErrorMessageInvalidNumber(); | ||
| throw new IllegalArgumentException(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
메소드명만 봐서는 어떤걸 검사하고 있는지 모르겠습니다. 좀 더 뚜렸한 네이밍으로 지어주세요
| public void printResult() { | ||
| System.out.println("당첨 통계"); | ||
| System.out.println("----"); | ||
| DecimalFormat decimalFormat = new DecimalFormat("#,###"); | ||
|
|
||
| // 상금 및 개수 출력 | ||
| for (String prize : prizeCounts.keySet()) { | ||
| int count = prizeCounts.get(prize); | ||
| int prizeAmount = getPrizeAmount(prize); // 해당 상금 가져오기 | ||
| String formattedPrizeAmount = decimalFormat.format(prizeAmount); // 상금을 쉼표가 있는 형식으로 변환 | ||
| System.out.println(prize + " (" + formattedPrizeAmount + "원) - " + count + "개"); | ||
| } | ||
| } |
There was a problem hiding this comment.
output을 담당하는 view에서 처리하는 것은 어떨까요?
| Lotto lotto = new Lotto(winningNumbers); // 유효성 검사 수행 | ||
| break; | ||
| } catch (IllegalArgumentException e) { | ||
|
|
There was a problem hiding this comment.
예외처리를 하지 않고 있는데, 의도한 것인가요?
src/main/java/lotto/model/Lotto.java
Outdated
|
|
||
| public class Lotto { | ||
| private final List<Integer> numbers; //로또 갯수 저장필드 | ||
| private static final int LOOTO_AMOUNT = 1000; |
There was a problem hiding this comment.
요구사항에 Lotto내에 새로운 필드를 생성하면 안된다는 조건이 있습니다. 요구사항을 지켜주세요
| for (Lotto lotto : purchasedLottos) { | ||
| matchCount = (int) lotto.getNumbers().stream() | ||
| .filter(winningNumbers.getNumbers()::contains) | ||
| .count(); | ||
|
|
||
| boolean hasBonus = lotto.getNumbers().contains(winningNumbers.getBonusNumber()); | ||
| lottoResult.updateResult(matchCount, hasBonus); // 결과 업데이트 | ||
| } |
There was a problem hiding this comment.
LottoResult 객체 내에 메소드로 분리하는 것은 어떤가요?
|
|
||
| private static List<Integer> winningNumberList; | ||
|
|
||
| public static int inputPlayerPrice() { |
There was a problem hiding this comment.
인스턴스를 생성하여 사용하는 것으로 알고 있는데, 메소드 전부에 static 키워드를 사용하고� 있는 것이 적절하지 않은 것 같습니다.
static 키워드를 어떤 경우에 사용되는 지, 사용하는 방법도 알아보시면 좋을 것 같습니다
| return Integer.parseInt(Console.readLine()); | ||
| } | ||
|
|
||
| //for문 서비스 쪽으로 옮겨야 하는데 못하겠네.. |
| System.out.println(ERROR_MESSAGE); | ||
| } | ||
|
|
||
| public static void printErrorMessageInvalidNumber() { |
There was a problem hiding this comment.
static 키워드가 적절한지 모두 다시 한번 생각해보시고 수정해주세요
|
다른 분들의 코드를 보고 많이 배우고 써먹을려고 한 점이 보입니다. 저는 석우님이 입출력 클래스안에 static 메소드를 사용해서 컨트롤러와 서비스간의 의존성을 줄일려는 구나라고 생각을 했습니다. 맞나요? 코드에 대한 피드백은 위에 멘토분들이 달아준 것을 따라가시면 좋을 것 같아요. 고생하셨습니다 |
과제를 완벽히 수행하지 못했습니다. 죄송합니다.