-
Notifications
You must be signed in to change notification settings - Fork 39
[김영현] Sprint4 #91
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 #91
The head ref may contain hidden characters: "Basic-\uAE40\uC601\uD604-sprint4"
Conversation
|
안녕하세요 주강사님! 이번 미션4를 구현하면서 가장 어려웠던 점은 기능 구현 자체보다 공통된 변수와 함수들을 효율적으로 관리하는 방법을 고민하는 것이었습니다.
이러한 방식으로 폴더 구조를 정리해도 괜찮을까요? 감사합니다!! |
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.
영현님 4번째 스프린트 미션 제출 수고하셨습니다~
깔끔하게 정리된 코드와 주석으로 의도를 잘 이해할 수 있었습니다!
이렇게 고민하시면서 여러가지 시도해보시는 것 너무 좋아요.
말씀하신 것처럼 기능 구현 자체보다 공통된 변수와 함수들을 효율적으로 관리하는 방법 를 고민할때 중요한 점은 효율성이라고 생각합니다. 그런데 효율이라는 것은 사실 굉장히 많은 요소들을 고려해서 판단해야하는 점이라 지금은 당연히 어려우실거에요~ 저도 어떻게 하는 것이 좋다고 말씀드릴 수 있다면 좋겠지만, 어떤 구조와 관리는 특정 시점에서는 오버 엔지니어링이 되기도 하고, 추후에는 좋은 설계가 되기도 합니다. 따라서 현 시점을 고려해서 결정을 하시고 추후 변경하시는 것이 제일 좋다고 생각해요~ (가능하면 미래를 예측해서 적절하게 대비하면 제일 좋지만 이는 시간과 경험이 필요한 일이라고 생각합니다!)
이러한 고민을 하시면서 점점 커지는 프로젝트를 설계해가시는 것이 스프린트 미션이라고 생각합니다~
다음 미션도 화이팅이에요!
-
공통 파일 관리 방식:
이는 위에서도 말씀드렸지만 그럴 수도 아닐 수도 있습니다. 지금은 이메일 패턴을 확인하는 정규식 하나만 있지만, 추후 정규식이 늘어난다면, 혹은 auth가 아닌 다른 페이지에서도 사용된다면 정규식만을 모아두는 파일을 따로 생성해 분리하는 것이 좋을거에요. 다만 지금의 경우는 이렇게 관리하셔도 큰 문제나 혹은 분리했을때의 장점이 비슷비슷할 것 같습니다. 이럴때는 취향의 문제라고 생각합니다! 이럴때저는 하나의 파일에서 관리하는 것이 좋다고 판단하여 작성하였습니다와 같이 스스로 판단하는 것이 중요하고 왜 그렇게 판단하셨는지 고민해보시고 프로젝트 규모나 요구사항이 변경되면 구조를 변경하시면 될 것 같습니다~ -
Netlify vs Vercel:
둘 다 배포할 수 있는 플랫폼이라는 점에서 동일합니다. 다만 netlify는 정적 사이트 배포에 많이 사용이 되고 vercel은 next를 만든 곳에서 만든 배포 플랫폼으로 next 프로젝트에 최적화가 되어 잇습니다. 지금과 같은 프로젝트의 경우는 netlfiy 면 충분합니다~ 관련 글들이 있으니 한번 읽어보세요! -
폴더 구조 변경:
특정 기준을 세우셔서 폴더 구조를 변경하신 점은 좋습니다~ 다만 auth에 account.css가 들어있고, common 폴더에는 common.css가 포함되어 있는 등 약간 폴더명으로 역할을 추측하기 어렵게 되어 있는 것 같습니다~ 폴더구조도 프로젝트 규모에 따라 결정되는 측면이 크기 때문에 저는 전의 구조가 지금 규모에서는 더 적절해보입니다~
학습이나 경험을 위해 구조를 변경해보고 싶으시다면 기준을 세워 분류해보시고, 이 프로젝트에 신입 개발자가 투입된다고 했을때를 상정해보시고 이해하기 쉬운지를 고민해보시면 좋을 것 같습니다.
| --white: #ffffff; | ||
| --background: #cfe5ff; | ||
| --skyblue: #e6f2ff; | ||
| --error-red: #f74747; |
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.
💊 제안
에러는 일반적으로 빨간색이므로 해당 색상변수명을 --error로 해도 될 것 같습니다~
| export const signupBtn = document.querySelector("#btn-signup"); | ||
|
|
||
| // 유효성 검사 패턴 | ||
| export const emailPattern = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; |
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.
💊 제안
이러한 패턴을 정규식이라고 합니다. 정규식을 작성하시면 이를 테스트해서 필요한 값들을 다 판별할 수 있는지 확인해보시는 것이 좋습니다~ 작성하신 것처럼 이메일 패턴 체크나 비밀번호 조건 체크할 때 사용합니다~ 변수가 많아지면 해당 변수들만 따로 분리하셔도 좋습니다~
| export let isEmailValid = { value: false }; | ||
| export let isPasswordValid = { value: false }; | ||
| export let isPasswordCheckValid = { value: false }; |
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.
❗️ 수정요청
위와 달리 이러한 변수들은 let으로 선언된 변경 및 재할당이 가능한 변수입니다.
한 파일에서 관리하기보다 사용하는 곳에서 선언해 관리하는 것이 흐름 파악 및 유지보수 측면에서 더 좋을 것 같습니다~
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 emailValue = emailInput.value.trim(); |
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.
💊 제안
주석을 다는 것은 좋은 습관입니다. 다만 주석 또한 읽어야 하는 정보라 중복되지 않는 내용을 입력해주시는 것이 가독성면에서 더 좋아요.
지금 주석은 아래 trim 메소드를 통해 파악할 수 있는 내용이라 없어도 될 것 같습니다~
| const loginBtnToggle = () => { | ||
| if (isEmailValid.value && isPasswordValid.value) { | ||
| loginBtn.style.backgroundColor = "#3578e5"; | ||
| loginBtn.style.cursor = "pointer"; | ||
| loginBtn.disabled = false; | ||
| } else { | ||
| loginBtn.style.backgroundColor = "#9ca3af"; | ||
| loginBtn.style.color = "#f3f4f6"; | ||
| loginBtn.style.cursor = "not-allowed"; | ||
| loginBtn.disabled = true; | ||
| } |
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.
💊 제안
해당 함수의 조건부분만 제외하면 signup-validation.js 파일의 signupBtnToggle과 동일한 함수네요~
해당 함수를 한번만 정의해서 두 파일에서 같이 사용할 수 있게 하시면 더 좋을 것 같아요~
이는 다른 로직들도 동일합니다~ 같은 로직의 경우 하나를 재사용하는 것이 유지보수 측면에서도 좋으니 account.css를 만드셨던 것처럼 두 파일의 중복을 줄이는 법도 고민해보세요~
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.
네 피드백 감사합니다. 저도 구현하면서 두 파일의 동일한 함수가 사용 되고 있어 이를 한번만 정의 해서 import하는 방식을 고민했는데 알려주셔서 감사합니다!!! 🔥🔥
기본 (로그인)
기본 (회원가입)
심화
주요 변경사항
배포 사이트
https://hyunbara4.netlify.app/
멘토에게