💫 Sprint Mission 4 Complete

[ghost]

요구사항

기본

• 기본 요구사항 구현 완료

심화

• []

주요 변경사항

스크린샷

멘토에게

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

[Taero-Kim]

p3;
단순히 @ 유무 여부로 이메일 값을 검사하기보다는 정확하게 이메일 패턴에 맞는지 확인하는게 좋을 것 같아요!
요럴때 정규표현식을 많이 사용하게 됩니다!

const emailPattern = /^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,6}$/;
...
else if (!emailPattern.test(emailInput.value)) {...}

[Taero-Kim]

p3;
목적에 맞게 기능을 잘 구현하신 것 같아요!
다만, 이 함수를 더 개선하기 위해서는 아래와 같은 것들을 고민해보면 좋을 것 같아요!

validateEmail는 현재 크게는 두 가지 일을 하고 있어요!
첫 번째는 이메일의 값을 통해 유효하지 않은 경우 errorMessage UI를 적용하고 있고,
두 번째는 유효성 여부의 결과 불리언 값을 리턴하고 있어요!

함수를 더 유지보수 및 재사용하기 용이하게 구성하려면, 더 작은 단위로 나누는게 좋아요!

우선 예시로

1. 불리언을 리턴하는 validate 부분
2. blur 되었을 때 UI를 변경하는 부분

이 2가지로 함수를 재구성해보겠습니다.

// 이메일 값의 유효성만 딱 검증하는 함수
const validateEmail = (emailValue) => {
  if (emailValue === '') return {isValid: false, message: '이메일을 입력해주세요.'};
  // --> 요렇게 조건에 따라 값을 리턴해버리면 함수가 종료되며 else나 else if 조건을 쓰지 않아도 되는 효과가 있습니다.
  // --> 이에 따라, 함수가 조금 더 깔끔해질 수 있어요!
  if (!emailValue.includes('@')) return {isValid: false, message: '잘못된 이메일 형식입니다.'};
  // --> 객체로 값을 리턴하는 이유는, 이렇게 했을 때, isValid 여부 뿐만이 아니라, isValid가 false인 경우에 변경되어야 하는 textContent 등도 한 번에 처리가능하여 좋습니다.
  return {isValid: true, message: ''};
}

// 이메일 값의 유효성에 따라 UI를 변경하는 부분
const �changeEmailInputStyleByValidation = () => {
  const eamilValidation = validateEmail(emailInput.value);

  if (eamilValidation.isValid) {
    emailError.textContent = '';
    emailError.style.display = 'none';
    emailInput.style.border = 'none';
    return;
  }

  emailError.textContent = eamilValidation.message;
  emailError.style.display = 'block';
  emailInput.style.border = '1px solid red';
}

emailInput.addEventListener('blur', changeEmailInputStyleByValidation);

[Taero-Kim]

p4;
요 부분도 한 번 제가 email에서 작성했던 내용을 보고 생각해보시고,
조금 더 작은 단위로 함수를 나누는 연습을 해보셔도 좋을 것 같아요!

[Taero-Kim]

p4;
요 부분도 위에서 제가 예시로 작성했던 코드를 기준으로 하면, 아래와 같이 사용할 수 있을 것 같아요!

const { isValid: isEmailValid } = validateEmail(emailInput.value);

[Taero-Kim]

p4;
요 부분도 조건문에 else를 붙여 다른 조건을 작성하는 대신, return으로 함수를 종료시켜서 else를 제거할 수 있습니다!
참고로 if, else if, else 방식으로 사용하는게 절대 틀린 방식이 아닙니다!
아주 자연스러운 코드입니다. 다만, 조금의 논쟁 여지가 있지만 일찍 return 시키는 패턴이 일반적으로는 더 깔끔하다고 간주되곤 합니다!
요걸 얼리 리턴 패턴이라고 해요!

if (isEmailValid && isPasswordValid) {
  loginButton.parentNode.href = '/items.html';
  return;
} 
  
loginButton.removeAttribute('href');

[Taero-Kim]

p3;
여기에 작성된 validateEmail과 valdiatePassword는 정확히 login.js에 작성되어 있던 함수와 동일하네요!

하나의 스크립트(js) 파일에서 함수를 분리하고 공통적으로 사용하는 것도 좋지만,
이렇게 여러개의 스크립트에서도 비슷한 기능들을 작성하게 되는 경우가 많습니다.

요런 경우, login과 signup에 공통적으로 사용되는 함수를 모아둔 스크립트 파일을 하나 만드시고,
공통적으로 사용하는 함수들을 export 하시고

각각의 login과 signup 함수에서 import 하셔서 사용하면 훨씬 좋을 것 같아요!

/* auth.js */
export validateEmail = () => {...}
export validatePassword = () => {...}

/*signup.js, login.js */
import { validateEmail, validatePassword } from './auth.js'

[Taero-Kim]

고생 많으셨어요, 준영님!
전반적인 기능을 잘 구현하신 것 같아요!
특히, 전반적으로 변수명을 이해하기 쉽게 잘 작명하신 것 같아요.
변수명을 직관적으로 작성하는 것은 코드의 가독성에 아주 도움이 많이됩니다!

다만, 함수를 조금 더 작은 단위로 나누는 연습을 하고,
공통되는 부분은 재사용할 수 있도록 공통 스크립트에 재사용되는 부분에 대한 함수를 정의하여
각각의 스크립트 파일에 가져다 사용하는 연습을 하시면 더 좋을 것 같아요!

지금은 함수를 작은 단위로 나누고, 공통되는 부분을 정의하여 분리하는게 조금 어색할 수 있지만,
요 부분은 앞으로 js로 함수를 많이 작성하시다 보면 자연스럽게 익숙해질거에요!

초반부에 들이는 이런 습관들이, 나중에 준영님 코드의 퀄리티를 훨씬 높여줄거에요!

이외에 코드를 보며 몇가지 개선하면 좋을 점에 대해서 간단한 리뷰들 남겼습니다!
제가 남긴 리뷰에 대한 질문이나 기타 질문이 있으시면 언제든 편하게 말씀해주세요!

+) 리뷰를 남긴 코멘트 위의 p(1~5); 마크의 의미는 아래와 같습니다! 참고하면 좋을 것 같아요!

p1; 꼭 반영해주세요 (Request changes)
리뷰어는 PR의 내용이 서비스에 중대한 오류를 발생할 수 있는 가능성을 잠재하고 있는 등 중대한 코드 수정이 반드시 필요하다고 판단되는 경우, P1 태그를 통해 리뷰 요청자에게 수정을 요청합니다. 리뷰 요청자는 p1 태그에 대해 리뷰어의 요청을 반영하거나, 반영할 수 없는 합리적인 의견을 통해 리뷰어를 설득할 수 있어야 합니다.

p2; 적극적으로 고려해주세요 (Request changes)
작성자는 P2에 대해 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장합니다.

p3; 웬만하면 반영해 주세요 (Comment)
작성자는 P3에 대해 수용하거나 만약 수용할 수 없는 상황이라면 반영할 수 없는 이유를 들어 설명하거나 다음에 반영할 계획을 명시적으로(JIRA 티켓 등으로) 표현할 것을 권장합니다. Request changes 가 아닌 Comment 와 함께 사용됩니다.

p4; 반영해도 좋고 넘어가도 좋습니다 (Approve)
작성자는 P4에 대해서는 아무런 의견을 달지 않고 무시해도 괜찮습니다. 해당 의견을 반영하는 게 좋을지 고민해 보는 정도면 충분합니다.

p5; 그냥 사소한 의견입니다 (Approve)
작성자는 P5에 대해 아무런 의견을 달지 않고 무시해도 괜찮습니다.