[2주차] 객체지향 코드 연습(xksoz)#54
Conversation
yunjin1213
left a comment
yunjin1213
left a comment
There was a problem hiding this comment.
구현 하시느라 수고하셨습니다!
애러 없이 잘 실행 되고 요구사항을 만족하는 것도 중요하지만, 이것들이 프로그래밍의 전부는 아니라고 생각합니다.
코드 구조 측면에서도 한 번 고민해보시면 더 완성도 높은 코드가 될 것 같아요.
아래에 글 하나 보내드립니다!
MVC 패턴이 무엇인지 한번 더 고민하는 시간을 가져보면, 더 성장하실 수 있을거에요.
| @@ -1,7 +1,134 @@ | |||
| package lotto; | |||
|
|
|||
There was a problem hiding this comment.
Application은 프로그램의 진입점(시작점) 역할만 해야 하지만, 지금은 그 이상의 역할을 담당하고 있습니다.
사용자 입력, 입력값 검증, 로또 생성, 로또 번호 출력 등 프로그램의 핵심 로직이 모두 몰려 있습니다.
이 구조는 클래스 분리가 안되서, 기능이 늘어날수록 Application만 계속 비대해지고, 유지보수가 힘들어집니다.
적어도 아래와 같이 분리하시면 좀 더 좋은 코드가 될 것 같습니다.
- input, output
- 로또 생성
- 당첨 결과 계산
- 등수 판정
- 검증 처리
가볍게 말씀드린거니, 구조에 대해 좀 더 고민해보세요!
| } | ||
| } | ||
|
|
||
| private static void start() { |
There was a problem hiding this comment.
전반적으로 매직넘버 처리가 안되어 있습니다.
ex) 1000, 1~45, 6, etc...
이러한 값들은 의미를 가지는 상수로 분리하면 가독성과 유지보수성이 더 좋아집니다~
아래와 같이 개선할 수 있을 것 같아요
private static final int LOTTO_PRICE = 1000;
private static final int LOTTO_SIZE = 6;
private static final int LOTTO_MIN_NUMBER = 1;
private static final int LOTTO_MAX_NUMBER = 45;
사용하는곳에서도 상수를 사용하는 형태입니다!
if (money % LOTTO_PRICE != 0) {
throw new IllegalArgumentException("[ERROR] 1,000원 단위로 입력해야 합니다.");
}
int count = money / LOTTO_PRICE;
lottos.add(new Lotto(Randoms.pickUniqueNumbersInRange(
LOTTO_MIN_NUMBER,
LOTTO_MAX_NUMBER,
LOTTO_SIZE
)));
| return nums; | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("[ERROR] 당첨 번호는 숫자여야 합니다."); | ||
| } |
|
|
||
| private static Map<LottoRank, Integer> calculateStats(List<Lotto> lottos, List<Integer> win, int bonus) { | ||
| Map<LottoRank, Integer> stats = new EnumMap<>(LottoRank.class); | ||
| for (Lotto lotto : lottos) { |
There was a problem hiding this comment.
등수 판정 로직이 Application에 있는 이유도 궁금합니다
| } | ||
|
|
||
| private static int countMatch(Lotto lotto, List<Integer> win) { | ||
| int count = 0; |
There was a problem hiding this comment.
결과 출력 로직이 Application에 직접 포함되어 있는데, View로 분리해보는건 어떨까요?
| } | ||
| } | ||
|
|
||
| private static List<Lotto> buyLottos(int money) { |
There was a problem hiding this comment.
input을 Application에서 담당하는게 맞을까요!?
한번 고민 해보세용
| @@ -1,20 +1,42 @@ | |||
| package lotto; | |||
|
|
|||
| import java.util.ArrayList; | |||
| import java.util.Collections; | |||
| import java.util.List; | |||
|
|
|||
| public class Lotto { | |||
There was a problem hiding this comment.
Lotto 클래스가 존재하지만, 현재 역할이 단순히 숫자 리스트를 만드는 수준에 머물러 있는 것 같습니다.
실제 로또 도메인을 목적으로 만드신거라면, Application의 로직들이 이곳에 위치해야 할 것 같습니다.
코드 구조에 대한 고민을 조금 더 해주시면 좋을 것 같아요
| @@ -0,0 +1,32 @@ | |||
| package lotto; | |||
|
|
|||
| public enum LottoRank { | |||
There was a problem hiding this comment.
Enum으로 나누신건 좋은 것 같아요.
또한 matchCount 필드만으로는 SECOND, THIRED를 구분할 수 없어 보이는데 이 값이 실제로 사용되고 있는지 한번 확인 해주시면 좋을 것 같아요
2 participants
No description provided.