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.

Sign up for GitHub

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

Jump to bottom

[클린코드 6기 홍석주] 로또 미션 STEP 3 #287

Open
wants to merge 3 commits into
base: hsju0202
Choose a base branch
from
Open

[클린코드 6기 홍석주] 로또 미션 STEP 3 #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e04a69
fix: checkLottos 유효성 검사 추가
hsju0202 May 26, 2024
f9aef1c
feat: web 기반 UI 개발
hsju0202 May 26, 2024
91997fd
docs: mod demo link
hsju0202 May 26, 2024
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</p>

<p align="middle">
<a href="https://next-step.github.io/js-lotto">🖥️ 데모 링크</a>
<a href="https://hsju0202.github.io/js-lotto/dist/">🖥️ 데모 링크</a>
</p>

## 🔥 Projects!
Expand Down
46 changes: 27 additions & 19 deletions index.html
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

로또 번호 input에 세자리 이상의 수도 입력이 가능한데 제한할 수 없을까요?

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<meta charset="UTF-8" />
<title>🎱 행운의 로또</title>
<meta name="viewport" content="width=device-width, initial-scale=1" />
<link rel="stylesheet" href="./src/css/index.css" />
</head>
<body>
<div id="app" class="p-3">
Expand All @@ -17,30 +16,35 @@ <h1 class="text-center">🎱 행운의 로또</h1>
</label>
<div class="d-flex">
<input
id="moneyInput"
type="number"
class="w-100 mr-2 pl-2"
placeholder="구입 금액"
/>
<button type="button" class="btn btn-cyan">확인</button>
<button id="buyButton" type="button" class="btn btn-cyan">
확인
</button>
</div>
</form>
<section class="mt-9">
<div class="d-flex">
<label class="flex-auto my-0">총 5개를 구매하였습니다.</label>
<label id="totalCountLabel" class="flex-auto my-0"></label>
<div class="flex-auto d-flex justify-end pr-1">
<label class="switch">
<input type="checkbox" class="lotto-numbers-toggle-button" />
<input
id="numbersToggle"
type="checkbox"
class="lotto-numbers-toggle-button"
/>
<span class="text-base font-normal">번호보기</span>
</label>
</div>
</div>
<div class="d-flex flex-wrap">
<span class="mx-1 text-4xl">🎟️ </span>
<span class="mx-1 text-4xl">🎟️ </span>
<span class="mx-1 text-4xl">🎟️ </span>
<span class="mx-1 text-4xl">🎟️ </span>
<span class="mx-1 text-4xl">🎟️ </span>
</div>
<div
id="lottosDiv"
class="d-flex flex-wrap"
style="display: none"
></div>
</section>
<form class="mt-9">
<label class="flex-auto d-inline-block mb-3"
Expand Down Expand Up @@ -84,6 +88,7 @@ <h4 class="mt-0 mb-3 text-center">보너스 번호</h4>
</div>
</div>
<button
id="resultButton"
type="button"
class="open-result-modal-button mt-5 btn btn-cyan w-100"
>
Expand Down Expand Up @@ -115,38 +120,41 @@ <h2 class="text-center">🏆 당첨 통계 🏆</h2>
<tr class="text-center">
<td class="p-3">3개</td>
<td class="p-3">5,000</td>
<td class="p-3">n개</td>
<td class="p-3"><span id="fifthRankCount"></span>개</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>
<td class="p-3"><span id="fourthRankCount"></span>개</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>
<td class="p-3"><span id="thirdRankCount"></span>개</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>
<td class="p-3"><span id="secondRankCount"></span>개</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>
<td class="p-3"><span id="firstRankCount"></span>개</td>
</tr>
</tbody>
</table>
</div>
<p class="text-center font-bold">당신의 총 수익률은 %입니다.</p>
<p class="text-center font-bold">
당신의 총 수익률은 <span id="profitRate"></span>%입니다.
</p>
<div class="d-flex justify-center mt-5">
<button type="button" class="btn btn-cyan">다시 시작하기</button>
<button id="restartButton" type="button" class="btn btn-cyan">
다시 시작하기
</button>
</div>
</div>
</div>
</div>
<script type="module" src="./src/js/index.js"></script>
</body>
</html>
9 changes: 6 additions & 3 deletions src/js/domain/LottoService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
LOTTO_DIGITS,
} from "./Lotto";
import { drawRandomItems } from "../util/Draw";
import { checkMoney } from "./LottoValidate";
import { checkMoney, checkLottos } from "./LottoValidate";

export const LOTTO_PRICE = 1_000;

Expand All @@ -21,8 +21,11 @@ export const buyLottos = (money) => {

export const getNumbersList = (lottos) => lottos.map((e) => e.numbers);

export const getLottoRanks = (lottos, winningNumbers, bonusNumber) =>
lottos.map((e) => e.compare(winningNumbers, bonusNumber));
export const getLottoRanks = (lottos, winningNumbers, bonusNumber) => {
checkLottos(lottos);

return lottos.map((e) => e.compare(winningNumbers, bonusNumber));
};

export const calculateProfitRate = (lottoQuantity, totalReward) =>
(totalReward / (lottoQuantity * LOTTO_PRICE)) * 100;
Expand Down
12 changes: 12 additions & 0 deletions src/js/domain/LottoValidate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,19 @@ export const checkMoney = (money) => {
}
};

export const checkLottos = (lottos) => {
if (!lottos) {
throw new Error("로또를 구입해주세요.");
}
};

export const checkNumbers = (numbers) => {
Copy link

Choose a reason for hiding this comment

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

지난 미션에서 놓친 부분같은데요, winningNumbers와 bonusNumbers에 중복된 번호가 있을 때 중복체크가 되지 않고 있어 수정 및 테스트 코드 추가가 필요해 보입니다

if (numbers.some((e) => !Number.isInteger(e))) {
throw new Error(
`로또(당첨) 번호는 ${LOTTO_MIN_NUMBER}~${LOTTO_MAX_NUMBER} 사이의 수입니다.`
);
}

if (numbers.length !== LOTTO_DIGITS) {
throw new Error(`로또(당첨) 번호의 자리수는 ${LOTTO_DIGITS}자리입니다.`);
}
Expand Down
24 changes: 12 additions & 12 deletions src/js/index.js
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

index.js보다 역할을 명시적으로 드러내는 파일명은 없을까요?

Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
const $showResultButton = document.querySelector('.open-result-modal-button')
const $modalClose = document.querySelector('.modal-close')
const $modal = document.querySelector('.modal')
const $showResultButton = document.querySelector(".open-result-modal-button");
const $modalClose = document.querySelector(".modal-close");
const $modal = document.querySelector(".modal");
const $lottoNumbersToggleButton = document.querySelector(
'.lotto-numbers-toggle-button'
)
".lotto-numbers-toggle-button"
);

const onModalShow = () => {
$modal.classList.add('open')
}
export const onModalShow = () => {
$modal.classList.add("open");
};

const onModalClose = () => {
$modal.classList.remove('open')
}
$modal.classList.remove("open");
};

$showResultButton.addEventListener('click', onModalShow)
$modalClose.addEventListener('click', onModalClose)
// $showResultButton.addEventListener("click", onModalShow);
Copy link

Choose a reason for hiding this comment

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

이 주석은 어떤 의미일까요?

$modalClose.addEventListener("click", onModalClose);
128 changes: 127 additions & 1 deletion src/step-web-index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,131 @@
* step 3의 시작점이 되는 파일입니다.
* 노드 환경에서 사용하는 readline 등을 불러올 경우 정상적으로 빌드할 수 없습니다.
*/
import "./css/index.css";
import "./js/index";
import {
buyLottos,
calculateProfitRate,
getLottoRanks,
} from "./js/domain/LottoService";
import { LottoStats } from "./js/domain/LottoStats";
import { LottoRank } from "./js/domain/enum/LottoRank";
import { onModalShow } from "./js/index";

console.log("Web Browser!");
const $buyButton = document.getElementById("buyButton");
const $moneyInput = document.getElementById("moneyInput");
const $totalCountLabel = document.getElementById("totalCountLabel");
const $lottosDiv = document.getElementById("lottosDiv");
const $numbersToggle = document.getElementById("numbersToggle");
const $winningNumberInputs = document.getElementsByClassName("winning-number");
const $bonusNumberInput = document.getElementsByClassName("bonus-number")[0];
const $resultButton = document.getElementById("resultButton");

const $fifthRankCountSpan = document.getElementById("fifthRankCount");
const $fourthRankCountSpan = document.getElementById("fourthRankCount");
const $thirdRankCountSpan = document.getElementById("thirdRankCount");
const $secondRankCountSpan = document.getElementById("secondRankCount");
const $firstRankCountSpan = document.getElementById("firstRankCount");
const $profitRateSpan = document.getElementById("profitRate");

const $restartButton = document.getElementById("restartButton");
Comment on lines +16 to +32
Copy link

Choose a reason for hiding this comment

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

DOM 조작, 접근, 브라우저 이벤트와 관련한 로직들은 index 파일에 작성하는 것보다는 view로 분리해버리면 좋을 것 같습니다 😄


let lottos = null;

$buyButton.addEventListener("click", buyLottosEvent);

$moneyInput.addEventListener("keydown", (event) => {
if (event.key === "Enter") {
event.preventDefault();
buyLottosEvent();
}
});
Comment on lines +36 to +43
Copy link

Choose a reason for hiding this comment

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

buyLottosEvent가 클릭 이벤트와 엔터키 입력 이벤트에 중복으로 등록되고 있네요. html form 요소를 사용하시면 엔터키 입력 시 자동으로 submit 이벤트가 발생해 직접 엔터키 입력이나 클릭 이벤트에 리스너를 달지 않아도 제출 로직을 구현할 수 있어요 😄

Suggested change
$buyButton.addEventListener("click", buyLottosEvent);
$moneyInput.addEventListener("keydown", (event) => {
if (event.key === "Enter") {
event.preventDefault();
buyLottosEvent();
}
});
$purchaseAmountform.addEventListener("submit", (event) => {
event.preventDefault();
buyLottosEvent();
});


$numbersToggle.addEventListener("change", (event) => {
const display = event.currentTarget.checked ? "" : "none";
$lottosDiv.style.display = display;
});

$resultButton.addEventListener("click", showLottoStats);

[...$winningNumberInputs].forEach((e) => {
Copy link

Choose a reason for hiding this comment

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

요소에 대한 인자명을 e로 하게 되면 event, error와 혼동될 여지가 많아 보여요

e.addEventListener("keydown", (event) => {
if (event.key === "Enter") {
showLottoStats();
}
});
});

$bonusNumberInput.addEventListener("keydown", (event) => {
if (event.key === "Enter") {
showLottoStats();
}
});

$restartButton.addEventListener("click", () => location.reload());
Copy link

Choose a reason for hiding this comment

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

참고) 새로고침으로 앱을 초기화하는게 간단하고 좋은 방법이지만 사용자 경험에는 좋지 않습니다

이번 미션에서 수정하실 필요는 없습니다!


function buyLottosEvent() {
try {
const money = getMoney();
Copy link

Choose a reason for hiding this comment

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

정수가 아닌 수를 입력해도 구매가 가능하네요 (1234.567)

lottos = buyLottos(money);
setTotalCountLabel(lottos.length);
setLottosDiv(lottos.map((e) => e.numbers));
$winningNumberInputs[0].focus();
Comment on lines +70 to +74
Copy link

Choose a reason for hiding this comment

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

자동 포커스 이동을 구현하셨군요 👍

} catch (err) {
alert(err.message);
}
}
Comment on lines +68 to +78
Copy link

Choose a reason for hiding this comment

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

이벤트에 대한 핸들러 함수명을 지을 때, handle~, on~과 같은 접두사를 이용해 표현하여 해당 로직을 담은 함수(예: buyLottos)와 구분하는 컨벤션을 많이 사용합니다.

만일 구입금액과 구입 버튼을 form으로 감싸고 submit 이벤트를 활용한다면 아래와 같은 네이밍이 되겠네요.

Suggested change
function buyLottosEvent() {
try {
const money = getMoney();
lottos = buyLottos(money);
setTotalCountLabel(lottos.length);
setLottosDiv(lottos.map((e) => e.numbers));
$winningNumberInputs[0].focus();
} catch (err) {
alert(err.message);
}
}
function handleSubmitPurchaseAmount(e) {
e.preventDefault();
try {
...
} catch (err) {
...
}
}


function showLottoStats() {
try {
const winningNumbers = getWinningNumbers();
const bonusNumber = getBonusNumber();

const lottoRanks = getLottoRanks(lottos, winningNumbers, bonusNumber);

const { rankCount, totalCount, totalReward } = new LottoStats(lottoRanks);

$fifthRankCountSpan.innerHTML = rankCount.get(LottoRank.FIFTH.rank) ?? 0;
$fourthRankCountSpan.innerHTML = rankCount.get(LottoRank.FOURTH.rank) ?? 0;
$thirdRankCountSpan.innerHTML = rankCount.get(LottoRank.THIRD.rank) ?? 0;
$secondRankCountSpan.innerHTML = rankCount.get(LottoRank.SECOND.rank) ?? 0;
$firstRankCountSpan.innerHTML = rankCount.get(LottoRank.FIRST.rank) ?? 0;

const profitRate = calculateProfitRate(totalCount, totalReward);
$profitRateSpan.innerHTML = profitRate;

onModalShow();
} catch (err) {
alert(err.message);
}
}
Comment on lines +80 to +102
Copy link

Choose a reason for hiding this comment

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

step-web-index.js에서 가장 긴 함수인데요, 함수 내부의 여러 관심사를 분리할 수 없을까요?


function getWinningNumbers() {
return [...$winningNumberInputs].map((e) => parseInt(e.value));
Copy link

Choose a reason for hiding this comment

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

parseInt에서 radix 생략 시 주의해주세요

}

function getBonusNumber() {
return parseInt($bonusNumberInput.value);
}

function getMoney() {
return parseInt($moneyInput.value);
}

function setTotalCountLabel(length) {
$totalCountLabel.innerHTML = `총 ${length}개를 구매하였습니다.`;
}

function setLottosDiv(numbersList) {
const spans = numbersList.map(generateLottoNumbersSpan);
$lottosDiv.replaceChildren(...spans);
}

function generateLottoNumbersSpan(numbers) {
const span = document.createElement("span");
span.classList.add("mx-1");
span.classList.add("text-4xl");
span.value = numbers;
span.innerHTML = `🎟️ ${numbers}`;
return span;
}