Skip to content
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

[2단계 - 행운의 로또 미션] 크리스(이송원) 미션 제출합니다. #54

Merged
merged 32 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
1b29e74
docs: step2 기능 & 테스트 항목 추가
swon3210 Feb 22, 2021
bcfb7df
refactor: class 기반 querySelector => id 기반 querySelector
swon3210 Feb 22, 2021
0441001
feat: modal 열고 닫기 기능 추가
swon3210 Feb 22, 2021
138152f
test: step 2 유저 입력 값 테스트 코드 구현
swon3210 Feb 22, 2021
8f7a174
test: step 2 기능 테스트 코드 구현
swon3210 Feb 22, 2021
b3015b1
feat: 당첨 번호와 보너스 번호를 입력
swon3210 Feb 22, 2021
0ff747f
feat: 결과 확인하기 버튼을 누르면 당첨 통계, 수익률을 모달로 확인
swon3210 Feb 23, 2021
d6981cf
feat: 다시 시작하기 버튼을 누르면 초기화 되서 다시 구매를 시작
swon3210 Feb 23, 2021
d5f43af
refactor: view에서 alert를 처리하도록 변경
swon3210 Feb 24, 2021
d04dc82
feat: 당첨 번호를 모두 입력하지 않았을 경우 에러 처리
swon3210 Feb 24, 2021
965b02f
test: not.exist => not.be.visible 로 변경
swon3210 Feb 24, 2021
9e3d93d
refactor: elements 선택자 상수화
swon3210 Feb 24, 2021
9022e46
refactor: init 메서드에서 hideResult 동작 분리
swon3210 Feb 24, 2021
506af7e
refactor: 메서드 및 변수명 변경
swon3210 Feb 24, 2021
045ead8
docs: 통과된 테스트에 체크 마크 추가
swon3210 Feb 24, 2021
8a63b01
Merge branch 'swon3210' of https://github.com/woowacourse/javascript-…
swon3210 Feb 24, 2021
69fecee
refactor: onCostSumbit => onCostSubmit 오타 수정
swon3210 Feb 25, 2021
f654044
refactor: validation 관련 파일 및 메서드 명 변경
swon3210 Feb 25, 2021
60c7dc4
refactor: view 메서드 명 변경
swon3210 Feb 25, 2021
48f83bd
refactor: LottoGame 관련 메서드 이름 변경
swon3210 Feb 25, 2021
c288e07
refactor: 바뀐 view, LottoGame 메서드명 service 에 반영
swon3210 Feb 25, 2021
51c8a8f
refactor: 문제가 없을 시의 check result 문자열 상수화
swon3210 Feb 25, 2021
56145e8
refactor: css class 문자열 상수화 및 적용
swon3210 Feb 25, 2021
0439909
refactor: 이름을 구분하여 getter 와 private, protected 속성을 더 잘 구분할 수 있게 변경
swon3210 Feb 26, 2021
48f39d0
docs: step3 기능 및 테스트 항목 추가
swon3210 Feb 27, 2021
2029389
feat: 로또 구매에 사용할 금액을 추가할 수 있음
swon3210 Feb 28, 2021
972cfd1
refactor: BEM CSS 작명 방법론 도입 & 테스트 코드를 도메인 별로 분리
swon3210 Mar 1, 2021
591d57b
refactor: LottoGame 클래스 내부의 변수와 메서드 순서를 변경
swon3210 Mar 1, 2021
c8e8697
refactor: Validation 을 위한 상수 객체 추가
swon3210 Mar 1, 2021
fc61f4f
refactor: number input 태그의 스타일링을 .input-box 클래스에서 일괄 관리
swon3210 Mar 1, 2021
c8861f8
fix: validator 메서드에 인자가 array 인지 검사하는 로직 추가
swon3210 Mar 1, 2021
dfa427d
fix: view, model, controller, utils 함수 모두에 예외처리 추가
swon3210 Mar 1, 2021
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
23 changes: 20 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@

### 🎯🎯 step2 당첨 결과 기능

- [ ] 결과 확인하기 버튼을 누르면 당첨 통계, 수익률을 모달로 확인할 수 있다.
- [ ] 로또 당첨 금액은 고정되어 있는 것으로 가정한다.
- [ ] 다시 시작하기 버튼을 누르면 초기화 되서 다시 구매를 시작할 수 있다.
- [x] 당첨 번호와 보너스 번호를 입력할 수 있다.
- 로또 번호들 중 중복된 번호가 있으면 안내메시지를 출력한다.
- 로또 번호들 중 1 ~ 45 사이의 숫자가 아닌 숫자가 있다면 안내메시지를 출력한다.
- [x] 결과 확인하기 버튼을 누르면 당첨 통계, 수익률을 모달로 확인할 수 있다.
- 당첨 번호와 보너스 번호를 입력하지 않으면 안내메세지를 출력한다.
- [x] 다시 시작하기 버튼을 누르면 초기화 되서 다시 구매를 시작할 수 있다.

### 🎯🎯🎯 step3 수동 구매

Expand All @@ -50,6 +53,13 @@
- [x] 소비자가 받은 각각의 복권에서 중복되는 숫자가 존재하면 안된다.
- [x] 금액은 1000원 이상을 입력해야 한다. 그 이하로 입력시 안내메세지를 출력한다.
- [x] 남는 금액이 있을 경우 남는 금액만큼을 빼도록 안내한다.
### step2 테스트

- [x] 입력된 번호들 중 중복된 번호가 있다면 안내메세지를 출력한다.
- [x] 입력된 번호들 중 1 ~ 45 사이의 숫자가 아닌 숫자가 있다면 안내메시지를 출력한다.
- [x] 당첨번호가 모두 입력되지 않으면 결과를 확인할 수 없다.
- [x] 결과 확인 버튼을 누르면, 당첨 통계, 수익률을 보여준다.
- [x] 다시 시작하기 버튼을 누르면, 로또게임이 초기화된다.

<br>

Expand Down Expand Up @@ -86,3 +96,10 @@ live-server 폴더명
## 📝 License

This project is [MIT](https://github.com/woowacourse/javascript-lotto/blob/main/LICENSE) licensed.

## 📓 Convention

- 클래스에서 속성명 앞에 _ 가 붙여진 것은 해당 속성이 protected 속성임을 뜻하며 인스턴스 외부에서 해당 속성을 수정해서는 안됩니다.
- 클래스에서 속성명 앞에 # 가 붙여진 것은 해당 속성이 private 속성임을 뜻하며 상속이 수행된 클래스 내부에서도 해당 속성을 사용하려 해서는 안됩니다.
- 어떤 인스턴스에 pascal case 표기된 속성이 있다면 해당 속성이 사실 getter 임을 뜻합니다. 이를 조작하려는 시도를 하지 않도록 유의해주십시오.

87 changes: 86 additions & 1 deletion cypress/integration/lotto.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('기능 테스트', () => {
cy.get('#lotto-count').should('have.text', '3');
cy.get('.lotto-item').should('have.length', '3');
});

it('소비자가 받은 각각의 복권에서 중복되는 숫자가 존재하면 안된다.', () => {
cy.get('#cost-input').type('5000');
cy.get('#cost-submit-button').click();
Expand All @@ -21,6 +22,40 @@ describe('기능 테스트', () => {
expect(lottoNumberList.length).to.equal(new Set(lottoNumberList).size);
});
});

it('결과 확인 버튼을 누르면, 당첨 통계, 수익률을 보여준다.', () => {
cy.get('#cost-input').type('3000');
cy.get('#cost-submit-button').click();
cy.get('.winning-number').eq(0).type(1);
cy.get('.winning-number').eq(1).type(2);
cy.get('.winning-number').eq(2).type(3);
cy.get('.winning-number').eq(3).type(4);
cy.get('.winning-number').eq(4).type(5);
cy.get('.winning-number').eq(5).type(6);
cy.get('.bonus-number').type(7);
cy.get('#result-modal-open-button').click();
cy.get('#modal').should('exist');
cy.get('#modal').contains('당첨 통계');
cy.get('#modal').contains('수익률');
});

it('다시 시작하기 버튼을 누르면, 로또게임이 초기화된다.', () => {
cy.get('#cost-input').type('3000');
cy.get('#cost-submit-button').click();
cy.get('.winning-number').eq(0).type(1);

Choose a reason for hiding this comment

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

일렬적인 패턴이 반복되고 있네요!

  1. 3000을 타이핑 한 후,
  2. 클릭
  3. 0번째에 타이핑을 1 하고 ~ 7까지 타이핑
  4. 모달 버튼 클릭

이러한 반복 행위는 함수로 빼도 괜찮지 않을까요~?

Copy link
Author

Choose a reason for hiding this comment

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

그렇네요...! 반영했습니다! 앞으로도 3개 이상 테스트에서 반복되는 2줄 이상의 코드는 함수로 만들어보겠습니다!

cy.get('.winning-number').eq(1).type(2);
cy.get('.winning-number').eq(2).type(3);
cy.get('.winning-number').eq(3).type(4);
cy.get('.winning-number').eq(4).type(5);
cy.get('.winning-number').eq(5).type(6);
cy.get('.bonus-number').type(7);
cy.get('#result-modal-open-button').click();
cy.get('#restart-button').click();
cy.get('#cost-input').should('have.value', '');
cy.get('#purchase-result').should('not.be.visible');
cy.get('#winning-number-input-form').should('not.be.visible');
cy.get('#modal').should('not.be.visible');
});
});

describe('유저 입력 값 테스트', () => {
Expand All @@ -30,6 +65,7 @@ describe('유저 입력 값 테스트', () => {
.then((win) => cy.stub(win, 'alert'))
.as('alertStub');
});

it('금액은 1000원 이상을 입력해야 한다. 그 이하로 입력시 안내메세지를 출력한다.', () => {
cy.get('#cost-input').type('500');
cy.get('#cost-submit-button').click();
Expand All @@ -39,6 +75,7 @@ describe('유저 입력 값 테스트', () => {
);
cy.get('#purchase-result').should('not.be.visible');
});

it('남는 금액이 있을 경우 남는 금액만큼을 빼도록 안내한다.', () => {
cy.get('#cost-input').type('3500');
cy.get('#cost-submit-button').click();
Expand All @@ -48,5 +85,53 @@ describe('유저 입력 값 테스트', () => {
);
cy.get('#purchase-result').should('not.be.visible');
});
});

it('입력된 번호들 중 중복된 번호가 있다면 안내메세지를 출력한다.', () => {
cy.get('#cost-input').type('3000');
cy.get('#cost-submit-button').click();
cy.get('.winning-number').eq(0).type(1);
cy.get('.winning-number').eq(1).type(2);
cy.get('.winning-number').eq(2).type(3);
cy.get('.winning-number').eq(3).type(5);
cy.get('.winning-number').eq(4).type(5);
cy.get('.winning-number').eq(5).click();
cy.get('@alertStub').should(
'be.calledWith',
MESSAGE.DUPLICATED_NUMBER_EXIST_MESSAGE
);
cy.get('.winning-number').eq(5).should('have.text', '');
});

it('입력된 번호들 중 1 ~ 45 사이의 숫자가 아닌 숫자가 있다면 안내메시지를 출력한다.', () => {
cy.get('#cost-input').type('3000');
cy.get('#cost-submit-button').click();
cy.get('.winning-number').eq(0).type(1);
cy.get('.winning-number').eq(1).type(2);
cy.get('.winning-number').eq(2).type(3);
cy.get('.winning-number').eq(3).type(4);
cy.get('.winning-number').eq(4).type(55);
cy.get('.winning-number').eq(5).click();
cy.get('@alertStub').should(
'be.calledWith',
MESSAGE.NUMBER_RANGE_EXCEEDED_MESSAGE
);
cy.get('.winning-number').eq(4).should('have.text', '');
});

it('당첨번호가 모두 입력되지 않으면 결과를 확인할 수 없다.', () => {
cy.get('#cost-input').type('3000');
cy.get('#cost-submit-button').click();
cy.get('.winning-number').eq(0).type(1);
cy.get('.winning-number').eq(1).type(2);
cy.get('.winning-number').eq(2).type(3);
cy.get('.winning-number').eq(3).type(4);
cy.get('.winning-number').eq(4).type(5);
cy.get('.bonus-number').type(6);
cy.get('#result-modal-open-button').click();
cy.get('@alertStub').should(
'be.calledWith',
MESSAGE.SHOULD_INPUT_ALL_NUMBERS_MESSAGE
);
cy.get('#modal').should('not.be.visible');
});
});
66 changes: 29 additions & 37 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ <h1 class="text-center">🎱 행운의 로또</h1>
</button>
</div>
</form>
<section id="purchase-result" class="mt-9 hide">
<section id="purchase-result" class="mt-9 removed">
<div class="d-flex">
<label id="purchase-item-count" class="flex-auto my-0"></label>
<div class="flex-auto d-flex justify-end pr-1">
Expand All @@ -41,62 +41,76 @@ <h1 class="text-center">🎱 행운의 로또</h1>
</label>
</div>
</div>
<div id="purchase-item-list" class="d-flex flex-wrap hide hide-lotto-numbers"></div>
<div
id="purchase-item-list"
class="d-flex flex-wrap removed lotto-numbers-removed"
></div>
</section>
<form class="mt-9" style="display: none">
<form id="winning-number-input-form" class="mt-9 removed">

Choose a reason for hiding this comment

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

id 네이밍이 굉장히 기네요! 하위에도 element 의 id가 길어지는 걸 볼 수 있구요!
BEM 방법론을 적용하여 특정한 규칙으로 만들면서 유효하게 관리하면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

BEM 을 적용하면 오히려 id 나 클래스가 길어지는 것으로 알고 있었는데 확실히 더 깔끔해진 것 같습니다! 그런데 조금이라도 더 짧게 하기 위해

check-number-form__input 이 아니라
check-number__input 이런 식의 생략을 추가한 것이 있는데 이 정도 생략은 괜찮은 것 같나요? 저는 개인적으로 해당 태그가 form 임을 클래스나 id 에서 명시해줘야 할 것이라고는 생각이 안들어서 일단은 뺐습니다!

Copy link

Choose a reason for hiding this comment

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

넵 좋은 것 같습니다!

<label class="flex-auto d-inline-block mb-3"
>지난 주 당첨번호 6개와 보너스 넘버 1개를 입력해주세요.</label
>
<div class="d-flex">
<div id="correct-number-wrapper" class="d-flex">
<div>
<h4 class="mt-0 mb-3 text-center">당첨 번호</h4>
<div>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
<input
type="number"
data-custom-input
class="winning-number mx-1 text-center"
/>
</div>
</div>
<div class="bonus-number-container flex-grow">
<h4 class="mt-0 mb-3 text-center">보너스 번호</h4>
<div class="d-flex justify-center">
<input type="number" class="bonus-number text-center" />
<input
type="number"
data-custom-input
class="bonus-number text-center"
/>
</div>
</div>
</div>
<button
type="button"
class="open-result-modal-button mt-5 btn btn-cyan w-100"
id="result-modal-open-button"
class="mt-5 btn btn-cyan w-100"
>
결과 확인하기
</button>
</form>
</div>
</div>

<div class="modal">
<div id="modal">
<div class="modal-inner p-10">
<div class="modal-close">
<div id="modal-close">
<svg viewbox="0 0 40 40">
<path class="close-x" d="M 10,10 L 30,30 M 30,10 L 10,30" />
</svg>
Expand All @@ -112,38 +126,16 @@ <h2 class="text-center">🏆 당첨 통계 🏆</h2>
<th class="p-3">당첨 갯수</th>
</tr>
</thead>
<tbody>
<tr class="text-center">
<td class="p-3">3개</td>
<td class="p-3">5,000</td>
<td class="p-3">n개</td>
</tr>
<tr class="text-center">
<td class="p-3">4개</td>
<td class="p-3">50,000</td>
<td class="p-3">n개</td>
</tr>
<tr class="text-center">
<td class="p-3">5개</td>
<td class="p-3">1,500,000</td>
<td class="p-3">n개</td>
</tr>
<tr class="text-center">
<td class="p-3">5개 + 보너스볼</td>
<td class="p-3">30,000,000</td>
<td class="p-3">n개</td>
</tr>
<tr class="text-center">
<td class="p-3">6개</td>
<td class="p-3">2,000,000,000</td>
<td class="p-3">n개</td>
</tr>
</tbody>
<tbody id="result-tbody"></tbody>
</table>
</div>
<p class="text-center font-bold">당신의 총 수익률은 %입니다.</p>
<p class="text-center font-bold">
당신의 총 수익률은 <span id="profit-rate"></span>%입니다.
</p>
<div class="d-flex justify-center mt-5">
<button type="button" class="btn btn-cyan">다시 시작하기</button>
<button id="restart-button" type="button" class="btn btn-cyan">
다시 시작하기
</button>
</div>
</div>
</div>
Expand Down
6 changes: 5 additions & 1 deletion src/assets/css/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ body {
padding-bottom: 4px;
}

#purchase-item-list.hide-lotto-numbers .lotto-numbers {
#purchase-item-list.lotto-numbers-removed .lotto-numbers {

Choose a reason for hiding this comment

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

display를 none하거나 display를 block으로 보이도록 설정하는 것을 따로 visible, invisible등의 css 로직으로 때내서 사용하는게 좀 더 좋지 않을까요? 그 때 그 때 마다 -removed 와 같은 긴 이름을 붙이는 것 보다 나아보여서요!

Choose a reason for hiding this comment

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

별도로, 위의 winning-number나 bonus-number의 width, height가 같아보이는데요! 이 친구들을 묶을 수 있는 하나의 클래스 단위를 만들어서 함께 쓰면 좋을 것 같습니다 :)

Choose a reason for hiding this comment

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

아래의 display.css에서 쓰고 있네요! 그걸 써보는 건 어떨까요? 요 로직은 필요없어보입니다!

Copy link
Author

Choose a reason for hiding this comment

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

winning-number나 bonus-number 에 적용할 스타일링은 input-box 라는 이름의 클래스에 넣어서 관리하도록 만들었습니다!

개인적인 생각으로는 저 lotto-numbers-removed 라는 클래스는 로또 아이템들이 안보이게 만드는게 아니라 로또 아이템 안의 로또 번호들만 안보이게 하는 동작에 사용되기 때문에, 정확히 어떤 동작이 일어나는지를 명시하기 위해서 어느 정도 길더라도 저렇게 명시하는 것이 필요하다고 생각했습니다. 다만 너무 길다는 생각이 들어서 lotto-numbers-removed => numbers-removed 로 변경하였습니다.

지적해주신 것처럼 #purchase-item-list 이 요소에 저런 조건식을 붙이지 않고 .lotto-numbers 요소들에 removed, added 라는 클래스를 넣고 붙이는 식으로 동작하는 방법이 더 간단하다는 말씀의 의도는 알겠습니다. 다만 그렇게 하면 모든 .lotto-numbers 요소들의 클래스를 일괄적으로 조작해주는 동작을 해야하는데, 그러면 .lotto-numbers 요소의 개수만큼 리플로우가 일어나지 않을까 하는 우려가 있습니다.

기존에 #purchase-item-list 요소에 클래스 하나를 넣고 붙이는 동작은 한번의 리플로우만 발생시키는 방식이라고 생각해서 렌더링 성능을 생각해보면 이쪽이 더 유효한 방법이 아닐까 하는 의견을 조심스럽게 내봅니다.

혹시 그 정도의 성능 차이는 크게 의미 없다고 생각하시는지, 제가 놓치고 있거나 제대로 이해하지 못한 부분이 있다면 지적해주시면 감사드리겠습니다.

Copy link

Choose a reason for hiding this comment

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

제가 생각하기에는, 성능이 무조껀 좋은 코드가 능사는 아닌 것 같아요. 해당 프로젝트에 맞게 함수를 만들고 설정하시면 될 것 같은데요~ 실시간 형태로 repaint, reflow가 몇십번 일어나지 않는 걸로 보이는데, 적용할 이유가 없어보입니다!

그리고 display가 reflow 의 문제가 있어보인다면, visibility나 opacity등으로 reflow의 해결도 할 수 있을 것 같습니다.

display: none;
}

#purchase-item-list.lotto-numbers-added .lotto-numbers {
display: '';

Choose a reason for hiding this comment

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

display: ''는 무얼 나타내는걸까요? display: block; 이 좀 더 명확하지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

빈 문자열로 만들면 가장 초기의 display 값으로 돌아간다는 말을 듣고 적용했던 것인데, 확실히 'block'이나 'initial'이라는 값으로 나타내는게 더 명시적이겠군요! 수정했습니다!

}

.lotto-item {
display: flex;
align-items: center;
Expand Down
1 change: 1 addition & 0 deletions src/assets/css/shared/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
@import 'modules/border.css';
@import 'modules/flex.css';
@import 'modules/display.css';
@import 'modules/visibility.css';
6 changes: 5 additions & 1 deletion src/assets/css/shared/modules/display.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.hide {
.block-added {

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.

감사합니다! 그런데 생각해보니 block-added 는 뭔가 이해가 잘 안될 수도 있는 이름 같기도 해서 block-added 를 d-block 으로 변경했습니다!

display: none;
}

.removed {
display: none;
}
7 changes: 7 additions & 0 deletions src/assets/css/shared/modules/visibility.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.visible {
visibility: visible;
}

.invisible {
visibility: hidden;
}
6 changes: 3 additions & 3 deletions src/assets/css/ui/modules/modal.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.modal {
#modal {
opacity: 0;
visibility: hidden;
display: flex;
Expand All @@ -12,7 +12,7 @@
z-index: 2;
}

.modal.open {
#modal.open {

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.

감사합니다!

opacity: 1;
visibility: visible;
}
Expand All @@ -27,7 +27,7 @@
position: relative;
}

.modal-close {
#modal-close {
margin: 20px;
width: 20px;
position: absolute;
Expand Down
Loading