[2단계 - 웹 기반 로또 게임] 클라우디 미션 제출합니다.#451
[2단계 - 웹 기반 로또 게임] 클라우디 미션 제출합니다.#451bel1c10ud wants to merge 109 commits intowoowacourse:bel1c10udfrom
Conversation
- 여러 개의 mock 데이터를 테스트 할 수 있게 하기 위함
- test-> moneyString
ukkodeveloper
left a comment
ukkodeveloper
left a comment
There was a problem hiding this comment.
안녕하세요 지오! 우코입니다.
우선 step2 까지 무사히 미션 제출하신 거 너무 수고 많으셨습니다. 스텝1의 구조를 살리면서 미션 진행한 점도 잘 느껴졌어요. 이제 실제 DOM을 붙이는 웹 UI작업을 했는데, 생각보다 자바스크립트로만 코드를 작성하려니 아마 불편한 점이 많았을 거라 생각해요 ㅎㅎ
Q&A
- 1단계 코드에서 View만 교체해 2단계 요구 조건 달성하기가 적합한 목표 였을지 궁금합니다.
저는 이 목표 자체는 충분히 잘 해내신 것 같아요. "도메인은 그대로 두고 UI만 갈아끼워 본다"였는데, 실제로 file changes를 보면 도메인 로직은 거의 그대로인 걸로 보이네요 ㅎㅎ
다만, 웹 UI로 가면서 유저 플로우가 조금 더 복잡해지거든요., 콘솔은 절차적으로 한 줄씩 입력을 받지만, 웹은 이벤트를 기다리다가 여러 입력을 한 번에 제출하잖아요. 그래서 WebInput이 폼 렌더링, 이벤트 연결, DOM 정리까지 함께 맡게 된 것 같아서 조금 역할이 커보입니다. (이것만 고민하고 스스로 정리만해도 정말 많은 걸 얻어갈 수 있다고 봐요!!)
- DOM 테스트와 E2E 테스트를 TDD 사이클에 맞춰 진행하는 것에 대한 어려움이 있었습니다.
E2E테스트는 보통 TDD로 하진 않는 것 같아요. 일단 실행하는 데에 시간도 걸릴 뿐더러, 효과도 그리 크지 않다고 생각합니다.
유닛 테스트의 경우는 작은단위로 나누는 점, 그리고 작은 단위를 계속 검증하면서 코드를 짤 수 있다는 점에서 효과정인데. E2E 테스트는 애초에 작은 단위가 아니라서요!!
너무 수고 많으셨고, 천천히 현재 코드를 다시 읽어보면서, 어떤 지점에서 코드 파악을 하는 데에 어려웠을 지. 그리고 이를 어떻게 해결할 수 있을지를 충분히 고민해보면 좋겠어요!!!
이 부분에 집중할 수 있도록 많은 피드백을 드리진 않았습니다. WebInput과 WebOutput 그리고 Output 까지. 이 부분을 중점적으로 개선해보면 좋을 것 같네요!
index.html
Outdated
| <div class="overlay hidden"> | ||
| <div class="modal"> | ||
| <header class="modal__header"></header> | ||
| <div class="modal__body"></div> | ||
| <footer class="modal__footer"></footer> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
모달이 step2의 핵심 UI인 만큼,
dialog 요소나 role="dialog"나 aria-modal 같은 의미도 같이 찾아보고 적용해봐도 좋을 것 같아요
| } | ||
| } | ||
|
|
||
| async readMoneyAsync() { |
There was a problem hiding this comment.
step2에서 넘어오면서 WebInput가 하는 일이 너무 많아보이는데요.
readMoneyAsync가 값을 읽는 것뿐 아니라 기존 DOM 정리, 폼 렌더링, submit 이벤트 연결까지 함께 맡고 있어서, 이름만 보면 실제 책임이 바로 들어오지는 않았어요!
당연히 step2에서 웹 UI를 붙이면서 복잡해진 건 자연스러운 일이지만...!
레이어를 어디까지 화면을 책임지는지 클라우디가 기준을 한 번 세워 두면 이후 메서드 이름도 훨씬 또렷해지고 역할 분리도 잘 될 것 같아요.
| formEl.className = "money__container"; | ||
| this.#elements.mainContainerBody.appendChild(formEl); | ||
|
|
||
| formEl.innerHTML = ` |
There was a problem hiding this comment.
innerHTML 을 사용하신 이유가 있을까요?!?!
장단점을 찾아보고 다른 대안들은 무엇이 있는지 정리해보면 좋을 것 같아요
| }); | ||
| } | ||
|
|
||
| async readWinningNumberAndBonusAsync() { |
There was a problem hiding this comment.
전반적으로 하나의 함수에서
- UI 로직
- 인터렉션 로직
- 도메인 로직
이 얽혀있는 것 같은데요! 어떻게 잘 분리할 수 있을까요?!
작게 실험해보면 좋을 것 같아서 readWinningNumberAndBonusAsync 함수만 분리해보는 것도 좋은 경험일 것 같습니다.
| class Output { | ||
| printError() { | ||
| throw new Error("printError 메서드가 구현되지 않았습니다."); | ||
| }; | ||
|
|
||
| printResult() { | ||
| throw new Error("printResult 메서드가 구현되지 않았습니다."); | ||
| }; | ||
|
|
||
| printPurchasedLottos() { | ||
| throw new Error("printPurchasedLottos 메서드가 구현되지 않았습니다."); | ||
| }; | ||
| } |
There was a problem hiding this comment.
error 을 throw한다는 것은 서비스 자체가 먹통이 될 수 있는 매우 강력한 동작이랍니다. 그거에 비해 현재 변수명은 print 라고 되어 있어요. 사용하는 측에서 너무 오해하기 쉽고 버그 발생 확률이 높아질 거에요!
| @@ -0,0 +1,15 @@ | |||
| class Output { | |||
There was a problem hiding this comment.
클라우디가 Output 객체를 따로 분리하여 상속구조를 만든 이유가 궁금해요
| alert(message); | ||
| } | ||
|
|
||
| printResult(countsObject, returnOnInvestment) { |
There was a problem hiding this comment.
역시 여기도 step2 오면서 하나의 메서드가 맡고 있는 책임이 너무 커지느 느낌이네요 ㅎㅎ
꼭 이 부분은 클라우디가 고민해보고 어떻게 하면 잘 분리할 수 있을지 고민해보면 좋을 것 같아요.
팁을 드리자면, 너무 거창하게 아키텍처나 설계를 한다기 보다는...
어떻게 비슷한 역할끼리 나누고, 어떻게 코드를 배치를 해야 한 흐름에 잘 읽히지? 를 고민해보면 좋을 것 같습니다.
| ${lottos | ||
| .map( | ||
| (lotto) => ` | ||
| <li class="purchased-lotto"> |
| margin: 84px auto; | ||
| min-height: 727px; | ||
| padding: 20px 15px; | ||
| width: 414px; |
There was a problem hiding this comment.
max-width나 반응형도 나중에 고려해보면 좋을 것 같네요 ㅎㅎ
3 participants
학습 목표
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
이번 2단계 미션의 제1 목표는
기존 1단계 코드에서View만 교체해 2단계의 요구 조건 달성하기였습니다.1단계에서 나름대로 도메인과 UI를 분리하여
View를 주입할 수 있는 형태로 설계했지만,막상 웹 환경을 붙이려다 보니 컨트롤러에 여전히 종속적인 UI 로직이 남아있었습니다.
2단계를 시작하며 우선 컨트롤러에 남아있던 UI 관련 로직을
View로 완전히 격리하는 리팩터링을 진행했습니다.이후 웹 환경에 맞는
WebInput과WebOutput을 새롭게 구현하며, Jest를 이용해 처음으로 DOM을 테스트 해보았는데요.여러 번의 시행착오가 있었지만, 원하는 테스트 케이스를 작성할 수 있었던 것 같습니다.
가장 여러웠던 부분은 '콘솔과 웹 환경의 입력 시점과 방식의 차이'였습니다.
절차적으로 입력을 받는 콘솔과 달리, 이벤트 기반으로 동작하는 웹 환경을 기존 컨트롤러로 대응하려다보니
일부 작위적으로 구현된 부분도 있는 것 같습니다.
2) 이번 리뷰를 통해 논의하고 싶은 부분
1.
1단계 코드에서 View만 교체해 2단계 요구 조건 달성하기가 적합한 목표 였을지 궁금합니다.결과적으로 동일한 App 인스턴스를 생성해 실행하되 View만 달리 주입해 콘솔과 웹 환경을 모두 대응하는것에 성공했으나
앞서 언급했던 것처럼 콘솔과 웹 환경의 차이로 인해 사용자의 입력 시점이나 방식에 있어 차이가 발생하다보니
일부 작위적으로 구현된 부분도 있는 것 같습니다.
사용자 시나리오가 달라져 컨트롤러를 새로 작성해야 할 것을
너무 무리해서 웹 환경에서 동작하도록 View를 끼워맞춘건 아닐지 궁금합니다.
2. DOM 테스트와 E2E 테스트를 TDD 사이클에 맞춰 진행하는 것에 대한 어려움이 있었습니다.
WebInput은 입력을 모킹하는 과정이 조금 까다로웠지만, 반환값이 문자열로 비교적 단순해 TDD 사이클을 온전히 지킬 수 있었는데요.반면
WebOutput에 대한 DOM 테스트나 전체 웹에 대한 E2E 테스트는 조금 막막했던 것 같습니다.구현을 시작하기 전 테스트를 먼저 작성하려다 보니, 머릿속으로 출력 형태가 그려지기는 했으나 마크업이 작성되지 않은 상태이기 때문에 이를 검증할 코드를 어떻게 짜야 할지 난감했습니다. 설사 마크업을 확정했더라도, 단순히 특정 문자열의 포함 여부만 확인하면 될지 아니면 마크업 구조 전반을 검증해야 할지에 대한 고민도 있었습니다.
이러한 어려움이 단순히 제가 익숙하지 않아서 겪은 것인지, 아니면 설계 및 실증 단계를 먼저 진행했어야 하는 것인지 궁금합니다.
✅ 리뷰어 체크 포인트
1단계
2단계