[이태경] sprint4 #123
[이태경] sprint4 #123
Conversation
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다!
제가 드린 코멘트처럼 재사용가능한 비즈니스 로직과 UI의 불필요한 결합만 제거한다면 코드가 훨씬 간결해지고 코드 양도 많이 줄어들수있을것같아요. 참고해보시고 리팩토링해보세요!
주요 리뷰 포인트
- 유지보수를 고려한 개발 (단일 책임 원칙)
- 네이밍
assets/css/member.css
Outdated
| } | ||
|
|
||
| /* validation 통과시 */ | ||
| .form__input-box.isPass .input { |
There was a problem hiding this comment.
NIT: isPass보다는 isValid 정도가 적당할것같네요!
assets/js/member.js
Outdated
|
|
||
| const VALIDATION_RULE = { | ||
| nickname: { | ||
| noValue: { |
There was a problem hiding this comment.
NIT: noValue => isEmpty 어떨까요?
복잡해보이는 네이밍을 기피하는것도 좋은 리팩토링이랍니다 :)
| function togglePassword(e) { | ||
| if (!e.target.closest(".input-box__toggle")) return; | ||
| const pwBox = e.target.closest(".input-box__input"); | ||
| const inputBox = pwBox.querySelector(".input"); | ||
| if (pwBox.classList.contains("pw_show")) { | ||
| pwBox.classList.remove("pw_show"); | ||
| inputBox.setAttribute("type", "password"); | ||
| } else { | ||
| pwBox.classList.add("pw_show"); | ||
| inputBox.setAttribute("type", "input"); | ||
| } | ||
| } |
There was a problem hiding this comment.
classList.toggle을 사용하면 좀더 간결하게 만들어줄수있겠네요 :)
참고
function togglePassword(e) {
if (!e.target.closest(".input-box__toggle")) return;
const pwBox = e.target.closest(".input-box__input");
const inputBox = pwBox.querySelector(".input");
const isShow = pwBox.classList.toggle("pw_show");
inputBox.setAttribute("type", isShow ? "text" : "password");
}
assets/js/member.js
Outdated
| } | ||
|
|
||
| // validation 검사 | ||
| function checkValidation({ target }) { |
There was a problem hiding this comment.
해당 함수는 비즈니스 로직과 UI가 불필요하게 결합되어있어 재사용이 불가하고, 하는일이 많아 복잡해보여요.
아래와 같이 분리해 단일 책임을 가질수있도록 리팩토링해볼까요?
- 유효성 검사 (추후 재사용 가능한 단위로 함수 분리해 모듈화)
- 이벤트 리스너 연결 및 UI 업데이트
assets/js/member.js
Outdated
| // 항목별 validation 검사 | ||
| switch (targetId) { | ||
| case "nickname": | ||
| isValidation = checkValidationNickname(targetValue); |
There was a problem hiding this comment.
위에서 얘기했던 코멘트처럼 비즈니스 로직과 UI를 분리하게된다면 이렇게 checkValidation{Nickname} 과 같이 nickname, password, email에 해당하는 개별 함수들을 더이상 따로 만들지않아도 되겠죠? :)
assets/js/member.js
Outdated
| if (!isValidation) { | ||
| // 항목별 validation 실패시 | ||
| createErrorMsg(VALIDATION_RULE[targetId].validation.msg, inputBox); | ||
| } else { | ||
| // 항목별 validation 통과시 | ||
| inputBox.classList.add("isPass"); | ||
| } | ||
| checkAllPass(); |
There was a problem hiding this comment.
이런 코드 흐름 너무 좋네요 👍 js에 대한 기본적인 이해도가 좋으신것같아요!
질문에 대한 답변
본문 내에서 상세히 답변드렸습니다! :) |
2 participants
요구사항
기본
로그인
회원가입
심화
스크린샷
로그인 페이지
입력값 없을 때
형식에 맞지 않는 값일 때
형식에 맞는 값일 때
input에 에러 메세지가 있을 때
회원가입 페이지
입력값 없을 때
형식에 맞지 않는 값일 때
형식에 맞는 값일 때
input에 에러 메세지가 있을 때
심화
멘토에게