-
Notifications
You must be signed in to change notification settings - Fork 39
[임재은] sprint4 #165
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.
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
[임재은] sprint4 #165
The head ref may contain hidden characters: "Basic-\uC784\uC7AC\uC740-sprint4"
Conversation
| // 이벤트 바인딩 | ||
| formElements.email.addEventListener("blur", () => { | ||
| handleValidation("email", formElements.email, formElements.warnings[0]); | ||
| }); | ||
|
|
||
| formElements.pw.addEventListener("input", () => { | ||
| handleValidation("pw", formElements.pw, formElements.warnings[1]); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 각 입력 필드마다 별도의 이벤트 리스너를 설정하고 있는데, 하나의 이벤트 핸들러로 통합하고 이벤트 위임(Event Delegation) 패턴을 사용하면 코드를 더 효율적으로 만들 수 있을것같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
먼저 폼 설정과 관련된 객체를 만들고, 초기화를 처리해서 관련있는 코드끼리 응집도를 높일수있게 처리해볼까요?
// 폼 설정 객체
const formConfig = {
email: {
selector: '#email',
warningIndex: 0,
validationType: 'email',
eventType: 'blur'
},
pw: {
selector: '#pw',
warningIndex: 1,
validationType: 'pw',
eventType: 'input'
}
};
// 폼 요소 초기화
const formElements = {
form: document.querySelector('form'),
warnings: document.querySelectorAll('.warning-text'),
eyeIcon: document.querySelector('.eye-icon'),
loginBtn: document.querySelector('.form-btn'),
fields: {}
};
// 폼 필드 초기화
Object.entries(formConfig).forEach(([key, config]) => {
formElements.fields[key] = document.querySelector(config.selector);
});There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고, 아래와 같이 하나의 이벤트 핸들러에서 위임 방식으로 이벤트를 처리해보면:
// 이벤트 리스너 설정
Object.entries(formConfig).forEach(([fieldName, config]) => {
const inputEl = formElements.fields[fieldName];
const warningEl = formElements.warnings[config.warningIndex];
inputEl.addEventListener(config.eventType, () => {
handleValidation(fieldName, inputEl, warningEl, config.validationType);
});
});개별 입력 필드마다 별도의 이벤트 리스너를 설정하지않아도되고,
이런 장점들이 생길 수 있어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개선으로 생길 수 있는 장점들
- 확장성: 새로운 폼 필드를 추가할 때 formConfig에 설정만 추가하면돼서, 확장에 용이해요.
- 각 필드의 설정이 명확하게 구조화되어 있어 유지보수에 용이해요.
- 이벤트 리스너 설정을 하나로 통합해두면, 이후 수정이 필요할 때 한 곳에서만 변경하면 되니까 추적도 쉬워지고 수정에도 용이해요.
- 각 입력 필드의 설정이 명확하게 분리되어 있어 디버깅이 쉬워져요.
- 코드 중복이 줄어들 수 있어요.
addiescode-sj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
리뷰하고 바로 머지 요청드린다고 본문에 남겨주셔서,
바로 머지해드릴테니 제가 남겨드린 코멘트는 잘 보시고 나중에 여유되실때 꼭 리팩토링 진행해보세요!
주요 리뷰 포인트
- 유지보수를 고려한 개발
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 파일에서도 위 login.js에서 드렸던 피드백 참고해보시고, 리팩토링 진행해볼까요? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각 함수가 적절한 수준으로 모듈화되어있네요. 굳굳! 좋습니다 :)
| // 이벤트 바인딩 | ||
| formElements.email.addEventListener("blur", () => { | ||
| handleValidation("email", formElements.email, formElements.warnings[0]); | ||
| }); | ||
|
|
||
| formElements.pw.addEventListener("input", () => { | ||
| handleValidation("pw", formElements.pw, formElements.warnings[1]); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
먼저 폼 설정과 관련된 객체를 만들고, 초기화를 처리해서 관련있는 코드끼리 응집도를 높일수있게 처리해볼까요?
// 폼 설정 객체
const formConfig = {
email: {
selector: '#email',
warningIndex: 0,
validationType: 'email',
eventType: 'blur'
},
pw: {
selector: '#pw',
warningIndex: 1,
validationType: 'pw',
eventType: 'input'
}
};
// 폼 요소 초기화
const formElements = {
form: document.querySelector('form'),
warnings: document.querySelectorAll('.warning-text'),
eyeIcon: document.querySelector('.eye-icon'),
loginBtn: document.querySelector('.form-btn'),
fields: {}
};
// 폼 필드 초기화
Object.entries(formConfig).forEach(([key, config]) => {
formElements.fields[key] = document.querySelector(config.selector);
});| // 이벤트 바인딩 | ||
| formElements.email.addEventListener("blur", () => { | ||
| handleValidation("email", formElements.email, formElements.warnings[0]); | ||
| }); | ||
|
|
||
| formElements.pw.addEventListener("input", () => { | ||
| handleValidation("pw", formElements.pw, formElements.warnings[1]); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고, 아래와 같이 하나의 이벤트 핸들러에서 위임 방식으로 이벤트를 처리해보면:
// 이벤트 리스너 설정
Object.entries(formConfig).forEach(([fieldName, config]) => {
const inputEl = formElements.fields[fieldName];
const warningEl = formElements.warnings[config.warningIndex];
inputEl.addEventListener(config.eventType, () => {
handleValidation(fieldName, inputEl, warningEl, config.validationType);
});
});개별 입력 필드마다 별도의 이벤트 리스너를 설정하지않아도되고,
이런 장점들이 생길 수 있어요.
| // 이벤트 바인딩 | ||
| formElements.email.addEventListener("blur", () => { | ||
| handleValidation("email", formElements.email, formElements.warnings[0]); | ||
| }); | ||
|
|
||
| formElements.pw.addEventListener("input", () => { | ||
| handleValidation("pw", formElements.pw, formElements.warnings[1]); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개선으로 생길 수 있는 장점들
- 확장성: 새로운 폼 필드를 추가할 때 formConfig에 설정만 추가하면돼서, 확장에 용이해요.
- 각 필드의 설정이 명확하게 구조화되어 있어 유지보수에 용이해요.
- 이벤트 리스너 설정을 하나로 통합해두면, 이후 수정이 필요할 때 한 곳에서만 변경하면 되니까 추적도 쉬워지고 수정에도 용이해요.
- 각 입력 필드의 설정이 명확하게 분리되어 있어 디버깅이 쉬워져요.
- 코드 중복이 줄어들 수 있어요.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게