[로또] 송아영 미션 제출합니다. #167
[로또] 송아영 미션 제출합니다. #167
Conversation
itwillbeoptimal
left a comment
itwillbeoptimal
left a comment
There was a problem hiding this comment.
3주차 미션도 고생하셨고 코드 잘 봤습니다 🙂
한 가지 궁금한 점이 있다면, 모델과 도메인 분리 기준을 어떻게 세우셨는지 궁금합니다. DrawnNumbers도 비즈니스 엔티티를 다루는 객체로 보이는데 모델로 분류하신 이유가 있을까요?
| #validatePurchaseAmount(purchaseAmount) { | ||
| if (purchaseAmount === "") { | ||
| throw new Error(PURCHASE_ERROR_MESSAGES.NONEMPTY); | ||
| } | ||
|
|
||
| if (purchaseAmount % PURCHASE_UNIT !== 0) { | ||
| throw new Error(PURCHASE_ERROR_MESSAGES.UNIT); | ||
| } | ||
| } |
There was a problem hiding this comment.
도메인이나 모델이 책임지는 값이 아니었기 때문에 컨트롤러에서 처리했습니다!
| Object.entries(RANK_TO_PRIZE_MAP).forEach(([rank, prize]) => { | ||
| const lottosInstance = new Lottos(PURCHASE_COUNT); | ||
| lottosInstance.win(rank); | ||
| const totalPrize = lottosInstance._getTotalPrize(); |
There was a problem hiding this comment.
지난 미션에서 아영 님께 제가 받았던 리뷰와 반대 상황이네요!
당시 제가 내부 동작 자체를 명확히 확인해야 한다면, private 필드를 직접 노출하지 않는 선에서 getter 메서드를 두는 방법도 있겠다고 했었는데, 테스트를 위해 내부 상태를 노출하는 것은 안티 패턴이 될 수도 있다고 하더라고요..😅 작성한 테스트가 동작이 아닌 구현을 검증하고 있는지 확인해 보는 것이 중요한 것 같습니다
There was a problem hiding this comment.
TDD를 시도하면서 단순히 메서드 구현 이전의 과정으로써 테스트를 작성했던 게 가장 큰 오류 같네요 🥲 성급한 적용이었던 것 같습니다. TDD의 단위를 어떻게 가져가야 할지 조금 감이 잡힐 것 같아요! 좋은 피드백 감사합니다~
| #winningNumbers; | ||
| #bonusNumber; | ||
|
|
||
| constructor(numbersString, bonusNumberString) { |
There was a problem hiding this comment.
모델에서 문자열 파싱까지 다루고 있는데, 컨트롤러나 별도 parser에서 변환 후에 생성자로 전달하는 게 더 깔끔하지 않을까요?
There was a problem hiding this comment.
어디까지가 모델의 책임이고, 어디서부터 협력해야 할지 기준을 제대로 세우지 못했던 것 같습니다 ㅠㅠ
처음에는 쉼표로 구분된 번호를 입력받는다는 입력 형식이나, 출력 형식까지도 모두 모델 내부 규칙이라고 판단해 파싱, 포매팅 로직도 모두 메서드로 구현했었는데, 데이터와 표현이 구분되어야겠다는 생각에 Formatter를 분리하게 되었거든요.
그 흐름을 이어가면 말씀 주신 것청럼 파싱 로직도 분리되는 게 자연스러웠을 것 같습니다!
| compare(anotherLotto) { | ||
| return this.#numbers.reduce( | ||
| (total, num) => (anotherLotto.includes(num) ? total + 1 : total), | ||
| 0 | ||
| ); | ||
| } |
There was a problem hiding this comment.
사실 로또 번호가 6개라서 큰 문제는 없지만, includes 대신 Set의 has를 사용하면 시간 복잡도를 O(n^2)에서 O(n)으로 줄일 수 있어요!
| UNIQUE: error("로또 번호는 중복될 수 없습니다."), | ||
| LENGTH: error(`로또 번호는 ${LOTTO_SIZE}개여야 합니다.`), | ||
| MIN_VALUE: error( | ||
| `로또 번호는 ${LOTTO_MAX_VALUE}에서 ${LOTTO_MAX_VALUE} 사이의 숫자여야 합니다.` |
There was a problem hiding this comment.
어라.. 여기는 LOTTO_MIN_VALUE여야 할 것 같아요 😅
@itwillbeoptimal 도메인은 문제의 핵심 개념을 표현하는 순수한 객체로 두고, 모델은 그 도메인을 중심으로 로직을 조합하거나 관리하는 상위 개념으로 생각했습니다. Lotto는 로또 한 장이라는 단위 개념을 표현하기 때문에 도메인으로 분류했고, Lottos와 DrawnNumbers는 등수나 수익률을 계산하는 등 비즈니스 규칙을 수행하는 로직을 포함하므로 모델로 분류하게 되었습니다! |
2 participants
폴더 구조
src ├─ constants │ ├─ errorMessages.js │ ├─ inputMessages.js │ ├─ lotto.js │ ├─ rank.js │ └─ unit.js ├─ controller │ └─ LottoController.js // 전체 로또 추첨 흐름 제어 ├─ domains │ └─ Lotto.js // 단일 로또 상태 관리 ├─ models │ ├─ DrawnNumbers.js // 당첨 번호 + 보너스 번호 상태 관리 │ ├─ LottoDrawer.js // DrawnNumbers -> Lottos 상태 업데이트 │ └─ Lottos.js // 사용자 로또 번호 상태 관리 ├─ utils │ ├─ error.js │ └─ Formatter.js // 출력문 형식 관리 └─ view ├─ InputView.js // 사용자 입력 처리 └─ OutputView.js // 출력 처리구현 목표
객체 지향
처음으로 객체 지향 프로그래밍에 도전해 보았습니다. 책임 분리를 어떻게 하고, 어떤 단위로 클래스들을 다시 묶어야 할지 고민이 많이 되었던 것 같습니다. MVC 패턴을 기반으로 하되, 각 데이터의 출력 형식을 Formatter에게 위임하는 방식으로 구현했습니다.
TDD
Lotto 클래스의 validate 테스트가 먼저 작성되어 있는 걸 보고 TDD 방식으로 구현하기를 기대했다고 판단, 적용해 보았습니다.
처음에는 TDD 방식을 따르는 게 하나의 기능을 구현할 때 고려할 게 많아 복잡성이 증가한다는 생각이 들었습니다. 테스트를 작성하면서 테스트 작성의 용이성에 대해 고려하고, 전체 설계를 재검토하는 시간들도, 구현을 빙빙 돌아간다고 느꼈습니다.
하지만 각 모델을 조합하는 controller를 구현하면서, TDD 방식을 따르며 개별 기능마다 설계를 돌아보고 리팩토링한 덕분에 정돈된 코드를 마무리할 수 있었다는 생각이 들었네요!