[나승엽] sprint4

[naseungyeop]

요구사항

기본

• [x]
• []
• []

심화

• [x]
• []

주요 변경사항

스크린샷

멘토에게

• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[GANGYIKIM]

승엽님 이번주차 스프린트 미션 제출하시느라 고생하셨습니다~
참고로 그전 멘토님께 리뷰받으셨거나, 파일 변경이 일어나지 않은 부분은 리뷰에 포함되지 않습니다.

아래는 전반적인 리뷰사항입니다.

---

• 동일한 사항에 대해서는 한번만 코멘트를 답니다. 비슷한 경우를 찾아서 변경해보세요~
• 로그인, 회원가입 페이지에서의 로직을 한 파일에서 처리하셨는데, 가능하시면 공통 사용하는 함수는 따로 분리하고, 로그인페이지의 JS파일과 회원가입페이지의 JS파일에서 해당 로직들을 가져다 쓰는 방식으로 작업하시면 코드 반복도 줄이고 가독성에도 좋을 것 같습니다.
• 다음번에 PR올려주실때는 PR 템플릿에 어떤 요구사항을 구현하셨는지 적어주시면 리뷰할 때 도움이 될 것 같습니다.
• PR 올리신 후 assignee말고 reviewr에 저를 할당해주세요~

[GANGYIKIM]

P2:
HTML 문서 다운로드 후 JS가 다운로드 되게 하단에 배치하신 것 같은데

하단에 배치된 스크립트는 구문분석 순서상 추후에 다운로드 되게 만들어주지만 일반적으로 스크립트 태그가 위치되는 head 요소가 아니므로 가독성 측면에서 단점이 있고 이러한 동작은 스크립트 자체 속성을 통해 이끌어 낼 수 있으므로 script async, defer 속성 사용을 추천드립니다.

https://developer.mozilla.org/ko/docs/Web/HTML/Element/script#defer
https://ko.javascript.info/script-async-defer

[GANGYIKIM]

P2:
안쓰는 코드는 가독성에 좋지 않습니다.
가능하면 지워주세요.

Suggested change

	/* PC: 1200px 이상 */
	@media (min-width: 1200px) {
	}

[GANGYIKIM]

P1:
이벤트를 연결하는 코드들이 함수 선언문 사이사이에 들어가 있어서 코드 파악이 어렵습니다.
가능하면 변수 선언 => 함수 선언 => 이벤트 바인딩문 과 같은 순서처럼 관련있는 코드끼리 기준을 정해서 묶어 작성하시는 것을 추천드립니다.

[GANGYIKIM]

P2:
해당 변수는 errorMessage가 들어갈 요소이니 해당 변수의 타입이 잘보이도록 아래와 같은 이름이면 더 좋을 것 같습니다.

Suggested change

	const emailError = document.getElementById("error_email");
	const emailErrorContainer = document.getElementById("error_email");
	const emailErrorElement = document.getElementById("error_email");

[GANGYIKIM]

P2:
emailStatus 라는 이름이 email input의 value를 나타내는지 잘 모르겠습니다.
아래와 같은 이름을 추천드립니다.

Suggested change

	const emilStatus = emailInput.value.trim();
	const emailValue = emailInput.value.trim();

[GANGYIKIM]

P2:
에러 메시지가 반복사용되니 변수로 저장해두고 사용해도 좋을 것 같습니다.

[GANGYIKIM]

P1:

Suggested change

	const verifypasswordStatus = verifypasswordInput.value.trim();
	const verifyPasswordStatus = verifypasswordInput.value.trim();

[GANGYIKIM]

P3:
이렇게 if 문의 조건이 길때는 해당 조건을 변수에 저장하시면 조금 더 가독성에 좋습니다.

Suggested change

	if (
	emilStatus !== "" &&
	emailValidate(emilStatus) &&
	passwordStatus !== "" &&
	passwordStatus.length >= 8 &&
	verifypasswordStatus === passwordStatus
	) {
	loginButton.classList.remove("disabled");
	const isValidForm = emilStatus !== "" &&
	emailValidate(emilStatus) &&
	passwordStatus !== "" &&
	passwordStatus.length >= 8 &&
	verifypasswordStatus === passwordStatus;
	if ( isValidForm ) {
	loginButton.classList.remove("disabled");

[GANGYIKIM]

P2:
어떤 값인지 알 수 있는 이름으로 해주세요~

Suggested change

	const re = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
	const regex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;