-
Notifications
You must be signed in to change notification settings - Fork 2k
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
mvc 연습 #2135
base: main
Are you sure you want to change the base?
mvc 연습 #2135
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.
고생했으
전체적으로 보면 도메인 로직이 조금 부족한거 그 부분 생각해서 도메인 모델에 로직과 서비스 컨트롤러의 로직을 분리 해보면 좋을것 같아
그리고 레파지토리가 Entity에 의해 생성된 DB에 접근하는 메소드거든 근데 해당 코드에서 디비를 사용하고 있지 않지만 도메인 로직에서 나온 결과를 저장하는것을 레파지토리를 이용해서 하고 걱기서 가져 올수있게 save, find 이거 두개만 해봐봐
처음하는거 치고 매우 잘함 굿굿!!
List<Integer> lottoNumbers = Randoms.pickUniqueNumbersInRange(1, 45, 6); | ||
lottoNumbers.sort(Integer::compareTo); | ||
lottoNumbersList.add(lottoNumbers); |
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.
불변 객체를 sorted 하고 있어서 문제가 발생하고 있음
실제로는 pickUniqueNumbersInRange로 값이 만들어지지만 테스트 상황에서는 임의 값을 넣어주고 있기 때문
List<Integer> lottoNumbers = Randoms.pickUniqueNumbersInRange(1, 45, 6); | |
lottoNumbers.sort(Integer::compareTo); | |
lottoNumbersList.add(lottoNumbers); | |
List<Integer> mutableNumbers = new ArrayList<>(lottoNumbers); | |
mutableNumbers.sort(Integer::compareTo); | |
lottoNumbersList.add(mutableNumbers); |
불변 객체를 가변 객체로 만들어 주면 통과~
private final LottoService lottoService; | ||
|
||
public LottoController(LottoService lottoService) { | ||
this.lottoService = lottoService; | ||
} |
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.
컨트롤러는 서비스 뿐만 아니라 view또한 외부 주입 받아야 하나 현재 의존성 주입이 안되고 있으
그냥 static 으로 열어버리는걸 지양해야해!
|
||
public void run(){ | ||
int purchaseAmount = InputView.getPurchaseAmount(); | ||
int numberOfLottos = purchaseAmount/1000; |
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.
컨트롤러에서 계산 로직이 있는것은 지양하는게 좋습니다
try { | ||
int amount = Integer.parseInt(purchaseAmount); | ||
if (amount <= 0) { | ||
throw new IllegalArgumentException("구매 금액은 0보다 커야 합니다."); | ||
} | ||
if (amount % 1000 != 0) { | ||
throw new IllegalArgumentException("구매 금액은 1000 단위로 나누어 떨어져야 합니다."); | ||
} | ||
} catch (NumberFormatException e) { | ||
throw new IllegalArgumentException("구매 금액은 숫자여야 합니다."); | ||
} |
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.
validation 은 크게 두가지로 나뉘어 입력단계 검증과 도메인 단계 검증
입력 단계 검증인 경우 잘못 입력하거나 이런것들 예시로들면 숫자인데 문자를 입력했을경우고
도메인 단계 검증은 해당 로직에서의 조건이야 도메인 단계 검증은 도메인에서 해주면 되고 입력단계검증은 view는 출력만을 위한것이기 때문에 따로 클래스를 만들어 분리하여야 해
public Map<String, Integer> calculateLotto(List<List<Integer>> lottoNumbers, Lotto winningLotto, int bonusNumber){ | ||
Map<String, Integer> lottoNumbersMap = new HashMap<>(); | ||
lottoNumbersMap.put("3개 일치", 0); | ||
lottoNumbersMap.put("4개 일치", 0); | ||
lottoNumbersMap.put("5개 일치", 0); | ||
lottoNumbersMap.put("5개 일치, 보너스 볼 일치", 0); | ||
lottoNumbersMap.put("6개 일치", 0); | ||
|
||
for (List<Integer> lottoNumber : lottoNumbers) { | ||
int matchCount = getMatchCount(lottoNumber, winningLotto.getNumbers()); | ||
boolean bonusMatch = lottoNumber.contains(bonusNumber); | ||
|
||
if (matchCount == 6) { | ||
lottoNumbersMap.put("6개 일치", lottoNumbersMap.get("6개 일치") + 1); | ||
} else if (matchCount == 5 && bonusMatch) { | ||
lottoNumbersMap.put("5개 일치, 보너스 볼 일치", lottoNumbersMap.get("5개 일치, 보너스 볼 일치") + 1); | ||
} else if (matchCount == 5) { | ||
lottoNumbersMap.put("5개 일치", lottoNumbersMap.get("5개 일치") + 1); | ||
} else if (matchCount == 4) { | ||
lottoNumbersMap.put("4개 일치", lottoNumbersMap.get("4개 일치") + 1); | ||
} else if (matchCount == 3) { | ||
lottoNumbersMap.put("3개 일치", lottoNumbersMap.get("3개 일치") + 1); | ||
} | ||
} | ||
return lottoNumbersMap; | ||
} |
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.
해당 내용은 도메인 로직으로 이동 시켜야 할것 같아
@Override | ||
public double calculateProfitRate(int purchaseAmount, Map<String, Integer> lottoNumberMap) { | ||
int totalWinningAmount = calculateTotalWinningAmount(lottoNumberMap); | ||
return (double) totalWinningAmount / purchaseAmount; | ||
} | ||
|
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.
수익률 계산이 잘못 되고 있어 전체 수악/사용 금액 * 100 이 되어야 수익률이 정상적으로 작동돼
또한 수익률을 담당하는 도메인 클래스로 따로 분리하는게 좋아보여 서비스 로직과 도메인 로직의 차이를 이해하는게 좋을것 같아 서비스 로직에서는 도메인 로직과 애플리케이션 계층 간의 조정 및 흐름 제어를 처리하는곳으로 나중에 디비 까지 쓴다면
도메인 로직을 호출하고, 이를 다른 계층(컨트롤러, 데이터베이스)과 연결 하는 역할을 해주어야해 직접적으로 값이 변하는 이러한 것들 즉 애플리케이션의 핵심 비즈니스 규칙은 도메인 모델 내부에서 이루저 져야해 객체지향적 코드에서는 도메인 모델이 자신의 값을 스스로 수정할 수 있어야해
while(true) { | ||
LottoDto winningNumbersDto = InputView.getWinningNumbers(); | ||
try { | ||
winningLotto = new Lotto(winningNumbersDto.getNumbers()); | ||
break; | ||
} catch (IllegalArgumentException e) { | ||
System.out.println("[ERROR] " + e.getMessage()); | ||
} | ||
} |
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.
트라이 캐치를 컨트롤러에서만 하면돼 여기서 입력 검증을 할 수 있는 클래스를 호출하고 다음에 도메인 모델 검증을 하게 한다면 컨트롤러 내부에서만 재입력 처리를 해주면 돼
public static int getBonusNumber(){ | ||
while (true) { | ||
try { | ||
System.out.println("\n보너스 번호를 입력해주세요."); | ||
String input = Console.readLine(); | ||
int bonusNumber = Integer.parseInt(input.trim()); | ||
if (bonusNumber < 1 || bonusNumber > 45) { | ||
throw new IllegalArgumentException("보너스 번호는 1과 45 사이의 숫자여야 합니다."); | ||
} | ||
return Integer.parseInt(input.trim()); | ||
} catch (IllegalArgumentException e) { | ||
System.out.println("[ERROR] " + e.getMessage()); | ||
} | ||
} |
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.
보너스 번호에 대한 예외 처리중 보너스 번호는 당첨 번호와 중복되지 않아야 한다가 누락되어 있어
numbers.add(Integer.parseInt(numberString.trim())); | ||
} | ||
} catch (NumberFormatException e) { | ||
throw new IllegalArgumentException("로또 번호는 숫자여야 합니다."); |
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.
에러 메시지를 정리해 놓는 클래스를 따로 두면 코드를 더 깔끔 하게 쓸 수 있고 재사용성이 올라가
|
||
import java.util.List; | ||
|
||
public class LottoDto { |
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.
dto는 데이터가 이동할때 한번 감싸서 넘겨줘 이것은 나중에 할건데
엔티티의 정보를 그대로 넘기면 외부 노출될수도 있고 그래서도 있고 필요한 특정 포맷에 맞출 수 있기도 해 이것말고도 찾아보면 여러 이유들이 있는데 해당 프로젝트에 맞춰서 어느부분에 dto를 사용할지 고려 하여 사용하면 될것 같아
No description provided.