Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/main/java/Application.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import controller.LottoController;

public class Application {

public static void main(String[] args) {
LottoController lottoController = new LottoController();
lottoController.run();
}
}
52 changes: 52 additions & 0 deletions src/main/java/controller/LottoController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package controller;

import model.Lotto;
import model.LottoTickets;
import view.InputView;
import view.ResultView;

import java.util.List;
import java.util.stream.Collectors;

public class LottoController {

public void run() {
try {
int purchaseAmount = InputView.getPurchaseAmount();

validatePurchaseAmount(purchaseAmount);

int ticketCount = Lotto.getTicketCount(purchaseAmount);

LottoTickets lottoTickets = new LottoTickets(ticketCount);

ResultView.printOrderTickets(ticketCount);
ResultView.printPurchasedLottoTickets(formatTickets(lottoTickets.getTickets()));
}catch (IllegalArgumentException e){
Copy link

Choose a reason for hiding this comment

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

공백이 필요해보여요

Copy link
Author

Choose a reason for hiding this comment

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

catch 앞에 공백을 추가하고, 메서드 호출문 사이에 공백을 추가하여 가독성을 높였습니다. 좋은 피드백 감사합니다!

            ResultView.printOrderTickets(ticketCount);

            ResultView.printPurchasedLottoTickets(formatTickets(lottoTickets.getTickets()));
            
        } catch (IllegalArgumentException e) {
        

ResultView.printErrorMessage(e.getMessage());
}
}

private List<String> formatTickets(List<Lotto> tickets) {
return tickets.stream()
.map(this::convertLottoToString)
.toList();
}

private String convertLottoToString(Lotto lotto) {
return lotto.getSortedNumbers().stream()
.map(String::valueOf)
.collect(Collectors.joining(",", "[", "]"));
}

private void validatePurchaseAmount(int purchaseAmount) {
if (purchaseAmount < Lotto.LOTTO_PRICE) {
throw new IllegalArgumentException("구매 금액은 1000원 이상이어야 합니다.");
}

if (purchaseAmount % Lotto.LOTTO_PRICE != 0) {
throw new IllegalArgumentException("구매 금액은 1000원 단위여야 합니다.");
}
}
Copy link

Choose a reason for hiding this comment

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

입력값 검증에 대해서는 정말 의견이 분분해요
정빈이는 model, view, controller중 어느 계층에서 검증이 이루어지는게 적절하다고 생각하나요?
정빈이 생각이 궁금해요

Copy link
Author

Choose a reason for hiding this comment

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

저는 검증은 Controller에서 이루어지는 것이 적절하다고 생각합니다.

  • Model은 데이터를 담당하므로 독립성을 유지해야 합니다.
    데이터 무결성을 보장하는 최소한의 검증만 Model에서 수행하고, 입력값 검증은 Controller에서 처리하는 것이 적절합니다.

  • View는 UI를 담당하므로 검증까지 포함하면 가독성이 떨어질 수 있습니다.
    View는 화면 출력과 입력만 담당해야 하며, 검증까지 맡으면 코드가 복잡해질 수 있습니다.

  • Controller는 Model과 View를 연결하는 역할을 하므로 검증을 담당하는 것이 적절합니다.
    Controller에서 입력값을 검증하여 Model에 올바른 데이터를 전달하면 유지보수성과 유연성이 높아집니다.

따라서, Controller에서 검증을 수행하여 상황에 맞는 데이터를 Model에 전달하는 것이 가장 적절한 구조라고 판단됩니다!


Copy link

Choose a reason for hiding this comment

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

불필요한 공백이 있어요

Copy link
Author

Choose a reason for hiding this comment

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

불필요한 공백을 삭제하였습니다. 좋은 피드백 감사합니다!

}
43 changes: 43 additions & 0 deletions src/main/java/model/Lotto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package model;

import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class Lotto {

// 로또 번호 관련 상수 선언
public static final int LOTTO_MIN_NUMBER = 1;
public static final int LOTTO_MAX_NUMBER = 45;
public static final int LOTTO_CREATE_SIZE = 6;
public static final int LOTTO_PRICE = 1000;
public static final List<Integer> LOTTO_NUMBER_POOL =
IntStream.rangeClosed(LOTTO_MIN_NUMBER,LOTTO_MAX_NUMBER)
.boxed()
.collect(Collectors.toList());

private List<Integer> numbers = new ArrayList<>();

public static int getTicketCount(int purchaseAmount){
return purchaseAmount / LOTTO_PRICE;
}
Copy link

Choose a reason for hiding this comment

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

만약 1000원단위로 금액이 입력되지 못하면 어쩌죠?

Copy link
Author

Choose a reason for hiding this comment

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

기능 요구사항에서 로또1장의 가격은 1000원이기 때문에, 1000원 단위 금액이 입력되지 않은 경우는 예외 처리가 필요합니다. 또한, 1000원 미만의 구매 금액이 입력되었을 경우에도 예외처리가 필요합니다. 피드백 반영하여 리팩토링 진행하였습니다. 감사합니다!

public void validatePurchaseAmount(int purchaseAmount) {
        if (purchaseAmount < Lotto.LOTTO_PRICE) {
            throw new IllegalArgumentException("구매 금액은 1000원 이상이어야 합니다.");
        }

        if (purchaseAmount % Lotto.LOTTO_PRICE != 0) {
            throw new IllegalArgumentException("구매 금액은 1000원 단위여야 합니다.");
        }
    }

Copy link

Choose a reason for hiding this comment

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

입력값에 따른 티켓 갯수를 반환하는것은 Lotto의 책임에 어색한 느낌을 받아요
로또 1장을 관리하는 model 안에 구입할 로또 티켓의 갯수를 계산하는 것 또한 어색해보이지 않았나요?
저번 리뷰에서 말씀 드린 내용과 비슷하게 Lotto 여러장을 관리하기 위한 동적 배열(List<Lotto>)의 갯수를 Lotto에서 관리하고 있는 느낌이에요

정빈이는 어떻게 생각해요?
고민해보고 정빈이의 생각을 알려주세요~

Copy link
Author

Choose a reason for hiding this comment

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

입력값에 따른 티켓 개수를 반환하는 것 역시 Lotto의 책임이 아닌 것 같습니다.

Lotto는 한 장의 로또를 관리하는 객체여야 합니다.
Lotto 객체는 단순히 6개의 숫자를 생성하고 관리하는 역할을 수행해야 합니다. 하지만 구매 금액을 기반으로 티켓 개수를 계산하는 로직이 포함되면, 단일 책임 원칙(SRP)이 깨질 수 있을 것 같습니다.

여러 개의 로또 티켓을 관리하고 개수를 계산하는 역할은 LottoTickets 에서 수행하는 것이 적절합니다. 이를 통해 Lotto는 한 장의 로또 관리에 집중하고, LottoTickets는 여러 장을 관리하는 역할을 분리할 수 있습니다.

만약 로또 구매 방식이 변하거나 추가 규칙이 생긴다면, 티켓 개수를 계산하는 로직은 LottoTickets에서만 수정하면 됩니다. 이렇게 하면 Lotto 클래스는 불필요한 변경 없이 안정적으로 유지할 수 있습니다.

따라서, Lotto는 한 장의 로또를 관리하는 역할만 수행하고, 티켓 개수 계산은 LottoTickets에서 담당하는 것이 더 적절하다고 생각합니다. 좋은 피드백 감사합니다!


public Lotto(){
this.numbers = createLottoNumbers();
}
Comment on lines +20 to +22
Copy link

Choose a reason for hiding this comment

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

자바 스타일 가이드를 참고하면

  1. 중첩 클래스
  2. 필드(멤버 변수)
  3. 생성자
  4. 메서드

순서를 제시하고 있어요 생성자의 순서가 조금 어색하죠? 수정해봅시다ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

초기화는 생성자에서 이루어지는 것이 적절하다고 생각합니다.

멤버 변수(numbers)는 생성자에서 초기화됩니다.

Lotto 클래스에서 numbers는 객체가 생성될 때마다 새로운 로또 번호를 가져야 하므로, 생성자 내부에서 this.numbers = createLottoNumbers(); 와 같이 초기화하는 것이 적절합니다.
초기화 로직은 별도의 메서드에서 처리하는 것이 가독성과 유지보수 측면에서 유리합니다.

createLottoNumbers() 메서드를 통해 초기화 로직을 분리하면, 생성자 내부가 간결해지고 코드의 역할이 명확해집니다.

불필요한 초기화를 제거하고 생성자에서 멤버 변수를 초기화하는 것이 자바 스타일 가이드에도 적합하며, 유지보수성과 가독성을 높일 수 있는 방법이라고 생각합니다.

좋은 피드백 감사합니다!


private List<Integer> createLottoNumbers(){
List<Integer> shuffledNumbers = new ArrayList<>(LOTTO_NUMBER_POOL);
Collections.shuffle(shuffledNumbers);
numbers = shuffledNumbers.subList(0, LOTTO_CREATE_SIZE);

return numbers;
}

public List<Integer> getSortedNumbers() {
List<Integer> sortedNumbers = new ArrayList<>(numbers);
Collections.sort(sortedNumbers);

return sortedNumbers;
}
}
25 changes: 25 additions & 0 deletions src/main/java/model/LottoTickets.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package model;

import java.util.ArrayList;
import java.util.List;

public class LottoTickets {

private final List<Lotto> tickets;

public LottoTickets(int ticketCount) {
this.tickets = generateLottoTickets(ticketCount);
}

private List<Lotto> generateLottoTickets(int ticketCount) {
List<Lotto> tickets = new ArrayList<>();
for (int i = 0; i < ticketCount; i++) {
tickets.add(new Lotto());
}
return tickets;
}

public List<Lotto> getTickets() {
return tickets;
}
Copy link

Choose a reason for hiding this comment

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

불변성 유지의 중요성에 대해서 알아가봅시다

List<Lotto>는 객체의 참조를 반환하고 있어요, 이렇게 반환된 객체의 참조는 다른 곳에서 수정될 가능성을 열어두고 있어요. 이에 대해서 방어적 복사 키워드에 대해서 공부해봅시다

충분히 고민, 공부해보고 왜 수정될 가능성을 열어두면 안되는지 정빈이의 생각을 알려주세요

Copy link
Author

@JeongBeanHyun JeongBeanHyun Mar 2, 2025

Choose a reason for hiding this comment

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

방어적 복사는 원본 데이터를 보호하고 불변성을 유지하기 위한 중요한 기법입니다.

현재 List의 참조를 직접 반환하면 외부에서 리스트를 수정할 가능성이 있습니다.
이렇게 되면 객체의 무결성이 깨질 수 있으며, 예기치 않은 오류가 발생할 가능성이 있습니다.
이를 방지하기 위해, 새로운 복사본을 만들어 반환하면 원본 데이터가 변경되지 않고 안전하게 보호됩니다.

public List<Lotto> getTickets() {
    return new ArrayList<>(tickets);
}

<방어적 복사의 장점>

  • 원본 데이터 보호 -> 외부에서 반환된 리스트를 변경하더라도, 원본 리스트는 변경되지 않습니다.

  • 예기치 않은 오류 방지 -> 데이터가 의도치 않게 변경되는 것을 방지하여, 디버깅을 쉽게 만들고 유지보수를 용이하게 합니다.

  • 멀티스레드 환경에서도 안전 -> 여러 스레드에서 같은 객체를 공유하더라도, 원본 데이터가 변경되지 않기 때문에 동기화 문제가 발생하지 않습니다.

좋은 피드백 감사합니다!

}
16 changes: 16 additions & 0 deletions src/main/java/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package view;

import java.util.Scanner;

public class InputView {

private static final Scanner scanner = new Scanner(System.in);

public static int getPurchaseAmount(){
System.out.println("구입금액을 입력해 주세요.");
int purchaseAmount = scanner.nextInt();
scanner.close();
Copy link

Choose a reason for hiding this comment

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

입력이 끝난 시점에서 바로 닫아주는 거 좋아용👍


return purchaseAmount;
}
}
20 changes: 20 additions & 0 deletions src/main/java/view/ResultView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package view;
import java.util.List;

public class ResultView {

public static void printOrderTickets(int ticketCount){
System.out.println();
System.out.println(ticketCount + "개를 구매했습니다.");
}

public static void printPurchasedLottoTickets(List<String> formattedTickets){
for (String ticket : formattedTickets) {
System.out.println(ticket);
}
}

public static void printErrorMessage(String message) {
System.out.println(message);
}
Copy link

Choose a reason for hiding this comment

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

에러메세지를 출력하는건 ResultView라는 클래스 즉 결과출력이라는 네이밍과 부합하게 보기에는 살짝 어색해보여요
정빈이는 어떻게 생각해요?

Copy link
Author

Choose a reason for hiding this comment

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

ResultView는 결과출력을 담당하기 때문에, 에러메세지 출력과는 거리가 있는 것 같습니다. ErrorView 라는 클래스로 분류하여 리팩토링 하였습니다!

}