Skip to content

Conversation

@Yoonjiho17
Copy link

3주차 과제 제출합니다!

Copy link

@ff1451 ff1451 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

script.js Outdated
Comment on lines 27 to 31
let number = new Set(); //중복 방지를 위한 set
while (number.size < 6) {
number.add(Math.floor(Math.random() * 45) + 1); //1~45 사이의 난수 생성
}
resultLottoNumber = [...number].sort((a, b) => a - b) //Set을 배열로 바꾼 후 오름차순 정렬
Copy link

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.

외부에 함수를 만들어서 그 함수를 불러와 변수에 넣는 식으로 재사용 가능하도록 수정해보겠습니다!

script.js Outdated
Comment on lines 40 to 45
$buyLottoButton.addEventListener('click', function() { //구매하기 클릭 이벤트
buyButtonCount++; // 구매하기 버튼 두 번 누르면 새로고침
if(buyButtonCount > 1) {
alert("다시 구매하세요!");
location.reload();
}
Copy link

Choose a reason for hiding this comment

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

연속 구매를 막기 위해 새로고침하는 방법보다는 disabled 속성을 이용해보는 것도 좋아 보여요

Copy link
Author

Choose a reason for hiding this comment

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

버튼을 비활성화하는 disabled 속성도 있다는 것을 알려주셔서 감사합니다!!

script.js Outdated
Comment on lines 57 to 65
function crossCheck(arr1, arr2) { //배열끼리 몇 개 일치하는지 비교하는 함수
let match = 0;
for(let i=0; i<6; i++) {
if (arr1[i] === arr2[i]) {
match++;
}
}
return match;
}
Copy link

Choose a reason for hiding this comment

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

현재의 검증 방식은 같은 숫자가 배열내에 있어도 인덱스가 다르면 같은 숫자를 체크하지 못하기 때문에 정확한 결과를 얻기 힘들어요

예를 들어 [1,2,3,4,5,6] 과 [4,5,6,7,8,9] 를 비교하면 match가 0이 반환돼요

Copy link
Author

@Yoonjiho17 Yoonjiho17 Apr 9, 2025

Choose a reason for hiding this comment

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

로또 당첨이 6개의 숫자와 순서 모두 일치해야하는 걸로 착각하고 있었습니다. 순서 상관 없이 숫자만 일치해도 match값이 오르는 반복문으로 고쳐보겠습니다!

$lottoResultText.innerText = `결과: ${matchCount}개 일치`
}

const inputvalue = parseInt($input.value); //입력값
Copy link

Choose a reason for hiding this comment

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

parseInt와 Number()의 차이는 무엇일까요?

Copy link
Author

Choose a reason for hiding this comment

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

parseInt()는 문자열에서 공백이 아닌 첫 문자부터 특정 진수의 정수를 반환합니다. 문자열에서 숫자 부분만 추출해서 정수로 변환하는데 숫자보다 문자가 앞에 있다면 NaN을 반환합니다.
그러나 Number()은 다른 타입(문자열, 불리언, null, undefined)의 값을 Number타입으로 바꾸는데에서 차이가 있습니다.

Copy link

@Yejin0070 Yejin0070 left a comment

Choose a reason for hiding this comment

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

과제하시느라 수고하셨습니다! 몇 가지 질문을 남겨보았으니 검토해보시면 좋을 것 같아요 ☺️

script.js Outdated
$container.style.height = `${1000 + height}px`;
$resultBox.style.height = `${460 + height}px`;
}
}); No newline at end of file

Choose a reason for hiding this comment

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

EOF, EOL 키워드에 대해 아실까요? 찾아보고 수정해봅시다. 또한, 자동 매번 설정하기 번거로우니 설정을 변경해보는 것도 좋을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

EOL은 줄 바꿈 문자이고 EOF는 파일의 끝이라는 의미로 파일의 마지막에 빈 줄 하나를 남겨서 해결했습니다. 자동 설정에 대해서도 찾아보겠습니다!

@@ -0,0 +1,72 @@
<!DOCTYPE html>
<html lang="en">

Choose a reason for hiding this comment

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

여기서 lang="en"과 "ko"의 차이가 무엇일까요?

Copy link
Author

Choose a reason for hiding this comment

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

lang="en"은 영어로 작성된 페이지이고
lang="ko"는 한국어로 작성된 페이지라는 뜻으로 검색 엔진, 음성 읽기, 번역 등에 언어를 인식시켜 줍니다.

script.js Outdated
Comment on lines 1 to 14
const $makeNumButton = document.querySelector("#makeNumButton");
const $resultNum1 = document.querySelector("#resultNum1");
const $resultNum2 = document.querySelector("#resultNum2");
const $resultNum3 = document.querySelector("#resultNum3");
const $resultNum4 = document.querySelector("#resultNum4");
const $resultNum5 = document.querySelector("#resultNum5");
const $resultNum6 = document.querySelector("#resultNum6");
const $buyLottoButton = document.querySelector("#buyLottoButton");
const $myNum1 = document.querySelector("#myNum1");
const $myNum2 = document.querySelector("#myNum2");
const $myNum3 = document.querySelector("#myNum3");
const $myNum4 = document.querySelector("#myNum4");
const $myNum5 = document.querySelector("#myNum5");
const $myNum6 = document.querySelector("#myNum6");

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.

변수 이름을 모두 다르게 해야 하다 보니 다 따로 선언해야 되는 줄 알았습니다. 배열과 루프를 사용해서 수정해 보겠습니다!

script.js Outdated

if(isNaN(inputvalue) || inputvalue <= 0) { //입력값 1이상 아니면 경고 및 새로고침
alert("구매 수량을 제대로 입력하세요!");
location.reload();

Choose a reason for hiding this comment

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

location.reload()로 전체 페이지 새로고침을 하고자 한 이유가 있을까요? input을 초기화 하거나 focus를 처리하거나 다른 방법도 있지 않을까요? 고민해봅시다!

Copy link
Author

Choose a reason for hiding this comment

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

사용자가 다른 값을 입력하였을 때 다시 입력하게 유도하기 위한 방법으로 경고와 새로고침을 하게끔 하였습니다. 새로고침을 하지 않으면 버튼을 눌렀을 때 실행되는 다른 코드가 같이 실행되어서 새로고침하는 방법을 사용했었습니다.
input초기화나 focus를 사용하고 구매 수량을 제대로 입력해야 다른 코드들이 실행되게끔 수정해보겠습니다!

Comment on lines +44 to +51
<div class="circleBox">
<span class="circle" id="resultNum1">?</span>
<span class="circle" id="resultNum2">?</span>
<span class="circle" id="resultNum3">?</span>
<span class="circle" id="resultNum4">?</span>
<span class="circle" id="resultNum5">?</span>
<span class="circle" id="resultNum6">?</span>
</div>

Choose a reason for hiding this comment

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

전체적으로 div, span으로 구성하신 것 같아요. 전체적으로 잘 짜주셨지만, 조금 더 다양한 태그를 사용해보는 것은 어떨까요? 예를 들어 이런 경우에는 ul li 태그들을 사용해볼 수 있을 것아요. 정답은 없지만, 조금 더 시멘틱한 코드를 짜기 위해 고민을 해보는것도 좋은 공부가 될 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

ul, li는 텍스트 정렬이라고만 생각했어서 이 코드에 사용해 볼 생각을 해보지 못했던 것 같습니다. 조언 감사합니다!

style.css Outdated
}

.headingText {
font-family: "Noto Sans KR", sans-serif;

Choose a reason for hiding this comment

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

밑에서 폰트를 반복해서 정의하고 있는데, 상위 요소에 선언하고 상속받는 방식은 어떨까요?

Copy link
Author

@Yoonjiho17 Yoonjiho17 Apr 9, 2025

Choose a reason for hiding this comment

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

제일 상위 요소인 body에 폰트를 정의해서 상속받도록 수정하겠습니다!

style.css Outdated
Comment on lines 145 to 146
width: 70;
height: 17;

Choose a reason for hiding this comment

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

px 단위가 빠진 것일까요? 아님 의도한 걸까요?

Copy link
Author

Choose a reason for hiding this comment

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

수정하다가 미처 지우는 걸 깜빡한 것 같습니다. 수정하겠습니다!

style.css Outdated
font-family: "Noto Sans KR", sans-serif;
font-weight: 700;
font-size: 16px;
} No newline at end of file
Copy link

@Yejin0070 Yejin0070 Apr 8, 2025

Choose a reason for hiding this comment

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

끝으로, 전체적으로 적용되지 않거나 상쇄되고 있는 스타일 코드가 있는지 검토해볼까요? 또한 반복 정의되고 있는 부분을 줄일 수 있는 방법에 대해서도 고민해보아요!

Copy link
Author

@Yoonjiho17 Yoonjiho17 Apr 9, 2025

Choose a reason for hiding this comment

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

넵 검토해보겠습니다. 조언 감사합니다!

style.css Outdated
@@ -0,0 +1,229 @@
body {
display:flex;
background-color: #f0f0f0;

Choose a reason for hiding this comment

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

사소하지만, 여러 군데에서 컬러 hex 값에서 소문자, 대문자를 섞어서 사용하고 있는 것 같아요. 소문자로 통일해볼까요?

Copy link
Author

Choose a reason for hiding this comment

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

넵! 코드를 작성할 때 통일성을 중요시하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants