Skip to content

Comments

Week02/jungan#12

Open
sandocsol wants to merge 4 commits intomainfrom
week02/jungan
Open

Week02/jungan#12
sandocsol wants to merge 4 commits intomainfrom
week02/jungan

Conversation

@sandocsol
Copy link
Contributor

✨어떤 과제를 수행했나요?✨

2주차 유튜브 홈페이지 클론코딩
자바스크립트로 캘린더 만들기

✨어려웠던점, 트러블슈팅✨

🤔 PR Point

캘린더 자바스크립트에서 겹치는 코드를 함수로 묶는게 까다로웠습니다. 잘 한건지, 더 정리할 게 있는지 궁금합니다.
어떨 때 Id를 쓰고 어떨 때 class를 써야 하는지 잘 모르겠습니다

Copy link
Member

@hyun907 hyun907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

유튜브 클론코딩에서 옆에 버튼 호버 스타일 주신 디테일 아주 좋습니다 ㅎㅎ 달력도 잘 구현해주셨어요!
특히 3개월 달력 구조를 깔끔하게 나눈 점, 그리고 각 달 렌더링을 하나의 renderCalendar() 함수로 재사용한 점이 정말 인상 깊었습니다!

전체 구조와 흐름이 명확해서 읽기도 편했고,
DOM 조작도 .innerHTML +=이 아니라 createElement, appendChild를 통해 성능 면에서도 안정적인 방식으로 잘 구현해주셨습니다 !!

특히 getBeforeAfterMonth() 함수로 이전/다음 달 계산을 따로 관리한 점도 코드의 의도를 잘 드러내는 좋은 구조인 것 같아요!


PR Point에 대한 답변 드리자면,

1. "겹치는 코드를 함수로 묶는 게 까다로웠다"는 점에 대해
사실 이 과제에서처럼 DOM 요소가 많고 달력 상태에 따라 매번 달라질 경우,
공통된 처리를 함수로 깔끔하게 묶는 건 쉽지 않습니다.

하지만 지금 작성하신 renderCalendar()는 상당히 잘 정리된 함수예요.
만약 더 정리해보고 싶다면, 아래 부분을 분리해볼 수 있어요!

  • <div> 생성 로직 → createEmptyCells(n)
  • 날짜 셀 생성 로직 → createDateCell(i, 조건들)

이렇게 세부 로직도 분리하면 더 유연하게 재사용 가능해질 것 같습니다.

2. "id와 class 언제 써야 할지 모르겠다"는 점에 대해

구분 언제 쓰는 게 좋은가? 예시
id 단 하나의 고유한 요소를 가리킬 때 #prevBtn, #calendarGrid
class 여러 개의 요소에 공통 스타일이나 처리를 적용할 때 .date, .calendar-dates

즉,

  • 버튼이나 헤더 같이 한 번만 존재하는 DOM 요소 → id
  • 날짜 셀처럼 반복되는 요소들 → class
    이런 기준으로 쓰면 될 것 같습니다.

전체적으로 코드의 구조화, 함수 분리, DOM 접근 방식 모두 매우 잘 짜여 있고
리팩토링도 스스로 고민하신 부분이 인상 깊었습니다.
이후에는 상태를 객체화하거나 캘린더 컴포넌트를 분리해보는 것도 좋은 도전이 될 것 같습니다!

Comment on lines +74 to +76
renderCalender(todayYear, todayMonth, currentMonthDates, currentMonthHeader);
renderCalender(beforeYear, beforeMonth, prevMonthDates, prevMonthHeader);
renderCalender(afterYear, afterMonth, nextMonthDates, nextMonthHeader);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 같은 함수가 호출이 3번되고 있는데, 한 가지 제안 드리자면,

이 부분은 하나의 renderAllMonths() 함수로 묶어서 가독성을 높여도 좋을 것 같아요!

function renderAllMonths() {
  getBeforeAfterMonth(); // 현재 todayMonth, todayYear 기준으로 before/after 계산

  renderCalendar(todayYear, todayMonth, currentMonthDates, currentMonthHeader);
  renderCalendar(beforeYear, beforeMonth, prevMonthDates, prevMonthHeader);
  renderCalendar(afterYear, afterMonth, nextMonthDates, nextMonthHeader);
}

버튼 클릭 시엔 이렇게 간단하게 호출할 수 있을 것 같습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants