Skip to content
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

6조 과제제출 (이은비, 주하림, 박현준) #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wngkfla01
Copy link

@wngkfla01 wngkfla01 commented Aug 10, 2023

온앤오프 (OnOff)

📋 프로젝트 소개

개발 기간 : 2023. 07. 24 ~ 2023. 08. 10

배포 링크 : 온앤오프

Github : 프론트엔드, 백엔드

👥 개발 팀원 및 역할

주하림 박현준 이은비
주하림 박현준 이은비
프로젝트 팀장
관리자 휴가/당직 관리 페이지
관리자 사원 관리 페이지
Navigation 메뉴바
로그인 페이지
회원가입 페이지
마이 페이지
초기 개발 세팅
일정 페이지
휴가 페이지
당직 페이지
엑셀 다운로드
휴가/당직 처리 알림 기능

⚒️ 기술 스택 및 개발 환경

Development

Config

Deployment

Environment

Cowork Tools

📌 프로젝트 기능 소개

API 설계

  • custom hook을 통해 axios interceptor를 활용하여 AccessToken을 자동으로 Headers에 설정하도록 구현
  • AccessToken 만료 시 interceptor를 통해 로그아웃 처리 및 로그인 페이지로 이동하도록 예외처리 구현

알림 기능

  • firebase firestore 연동을 통해 관리자가 휴가 및 당직의 승인/반려 처리 시 알림 기능 추가
  • 알림 읽음(선택) 시 알림 읽음 처리 및 처리 상태를 확인 하기 위한 페이지로 이동 구현

로그인 / 회원가입 Page

로그인 Page

  • Email 유효성 검사
  • 회원가입 링크
  • 로그인 시 유저정보 및 토큰 저장
  • 로그인 -> 홈화면 이동

회원가입 Page

  • 기본적인 유효성 검사(필드 전체 입력)
  • Email,핸드폰,비밀번호 확인 유효성 검사
  • 중복값 존재 등 API 다양한 통신 에러 예외처리 기능
  • 회원가입 -> 로그인화면 이동

내 정보 Page

  • 로그인 시 저장된 유저값을 가져와서 사용
  • 정보 수정 시 유효성 검사 기능
  • 중복값 존재 등 API 다양한 통신 에러 예외처리 기능
  • 전화번호 변경 기능(버튼(수정<->완료) 전환 시 수정가능)
  • 비밀번호 변경 기능(모달)

홈 - 일정 Page

  • 캘린더를 통한 월별 사원들의 휴가/당직 일정 조회
  • 사용자 선택 시 해당 사용자의 일정만 조회 가능
  • 엑셀 다운로드 시 현재 캘린더로 조회하고 있는 일정을 엑셀 파일로 다운로드
  • 캘린더 헤더와 스타일 커스텀 (주말 색상 구분 등)

휴가/당직 신청 조회

휴가 Page

휴가 조회

  • 사용자의 휴가일수 조회 및 사용한 휴가일수 계산을 통한 휴가일수 현황 표시
  • 사용자의 휴가 신청 내역을 테이블 형태로 조회
  • 사용자의 휴가 사용 내역을 테이블 형태로 조회
  • 테이블의 정렬 또는 필터 기능 추가
  • 신청한 휴가 취소 기능
  • 휴가 조회 실패 및 취소 실패 등 다양한 API 통신 에러 예외처리 추가

휴가 신청

  • 연차 / 오전 반차 / 오후 반차 타입 제공
  • 휴가 일정 선택 시 주말 제외
  • 휴가 타입에 따른 휴가 일정 유효성 검사 추가
  • 신청가능한 휴가일수에 따른 유효성 검사 추가
  • 휴가 신청 API 통신 에러 예외처리 추가

당직 Page

당직 조회

  • 사용자의 당직 신청 내역을 테이블 형태로 조회
  • 사용자의 당직 기록 내역을 테이블 형태로 조회
  • 테이블의 정렬 또는 필터 기능 추가
  • 신청한 당직 취소 기능
  • 당직 조회 실패 및 취소 실패 등 다양한 API 통신 에러 예외처리 추가

당직 신청

  • 당직 일정 선택 시 주말 제외
  • 당직 신청 API 통신 에러 예외처리 추가

관리자

휴가/당직 관리 Page

휴가/당직 신청현황 조회

  • 모든 사용자의 휴가/당직 신청현황 테이블 형태로 조회
  • '승인/반려'처리를 하지 않은 신청 내역을 가장 상단에 오도록 정렬 기능 추가
  • 각 테이블 column별로 필터링 기능 추가
  • Pagination기능 추가

휴가/당직 승인/반려

  • 모든 신청 내역을 '승인' 또는 '반려' 버튼으로 처리(상태값 변경)
  • Popconfirm 메시지 기능을 추가해 실수로 클릭해 처리하는것 방지

사원 관리 Page

사원 조회

  • 모든 User정보를 테이블 형태로 조회
  • 각 테이블 column별로 필터링 기능 추가
  • Pagination기능 추가
  • 이름 또는 이메일로 사원 검색 기능 추가

사원 정보 수정

  • 테이블에서 특정 사용자의 해당 row 클릭 시 Modal창으로 사용자의 상세 정보 표출
  • 특정 정보(직급, 권한, 연락처) 수정 기능 추가
  • 연락처 입력 Input 숫자만 입력 가능 및 자릿수 체킹 기능 추가

📂 프로젝트 구조

접기/펼치기

📦src
┣ 📂apis
┃ ┣ 📜axios.ts
┃ ┗ 📜index.ts

┣ 📂components
┃ ┣ 📂admin
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂calendar
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂common
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂dayoff
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂duty
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂login
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂mypage
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂signup
┃ ┃ ┗ 📜index.ts
┃ ┗ 📜index.ts
┣ 📂constants
┃ ┗ 📜index.ts
┣ 📂hooks
┃ ┗ 📜index.ts
┣ 📂pages
┃ ┣ 📜AdminEmployee.tsx
┃ ┣ 📜AdminSchedule.tsx
┃ ┣ 📜App.tsx
┃ ┣ 📜DayOff.tsx
┃ ┣ 📜Duty.tsx
┃ ┣ 📜HomeCalendar.tsx
┃ ┣ 📜Login.tsx
┃ ┣ 📜MyPage.tsx
┃ ┣ 📜SignUp.tsx
┃ ┗ 📜index.ts
┣ 📂stores
┃ ┗ 📜index.ts
┣ 📂types
┃ ┣ 📂admin
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂calendar
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂common
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂dayoff
┃ ┃ ┗ 📜index.ts
┃ ┣ 📂duty
┃ ┃ ┗ 📜index.ts
┃ ┣ 📜IUser.ts
┃ ┗ 📜index.ts
┣ 📂utils
┃ ┗ 📜index.ts
┣ 📜GlobalStyle.ts
┣ 📜GlobalThemeConfig.ts
┣ 📜firebase.ts
┣ 📜main.tsx
┗ 📜vite-env.d.ts

🖼️ 결과물

로그인 페이지 회원가입 페이지
로그인 페이지 회원가입 페이지
홈 페이지(일반) 홈 페이지(관리자)
홈 페이지 홈 페이지 관리자
알림 조회 일정 상세 조회
image image
내 정보 페이지 비밀번호 변경 페이지
내 정보 페이지 전화번호 변경 내 정보 페이지 비밀번호변경
휴가 페이지 휴가 등록
image image
당직 페이지 당직 등록
image image

|

관리자 휴가/당직 관리 관리자 사원 관리
휴가 관리 페이지 사원관리 페이지
사원 정보 조회 및 수정 엑셀 다운로드
사원관리 페이지 사원 세부 정보 액셀 다운

@wngkfla01 wngkfla01 changed the title 6조 과제제출 6조 과제제출 (이은비, 주하림, 박현준) Aug 10, 2023
@wngkfla01 wngkfla01 self-assigned this Aug 10, 2023
kse-seong-eun added a commit that referenced this pull request Aug 11, 2023
Copy link

@azure0929 azure0929 left a comment

Choose a reason for hiding this comment

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

미니프로젝트 진행 하시느라 고생 많으셨습니다!
전체적으로 '온앤오프'의 느낌이 디자인에 잘 반영이 되어있고, 구현을 너무 잘하셨습니다.
각 페이지별로 어떤 기능을 구현을 해야하는지
팀원분들과 고민을 많이 하신 부분들이 많이 보인 프로젝트인 것 같습니다.
인상깊었던 부분들과 궁금한 부분들을 코멘트 남겨봤습니다!
이번 파이널 프로젝트에서도 모두 파이팅하세요!

Comment on lines +12 to +16
export const loginRequest = async (params: ILoginData): Promise<IDataResponse<ILoginUser>> => {
const response = await client.post('/login', params)
return response.data
}

Choose a reason for hiding this comment

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

params를 사용하여 서버로부터 받은 응답 데이터를 반환하고, 응답 데이터 객체의 프로미스를 사용한 부분이 인상깊었습니다!

Comment on lines +5 to +10
export const client = axios.create({
baseURL: host,
headers: {
'Content-Type': 'application/json'
}
})

Choose a reason for hiding this comment

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

apis 폴더 안에 api.ts를 생성하지 않고 axios.ts를 따로 만드신 특별한 이유가 궁금합니다!

Copy link

Choose a reason for hiding this comment

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

axios.ts는 axios config를 설정하는 코드가 있는 부분이라 파일명을 axios라고 지었습니다!

@@ -0,0 +1,47 @@
import { Skeleton, SkeletonProps, Table } from 'antd'

Choose a reason for hiding this comment

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

'antd' 라이브러리를 사용하여 스켈레톤 로딩 상태에 따라 스켈레톤 테이블 또는 실제 데이터를 표시한 부분이 인상깊었습니다. 'antd' 라이브러리를 사용하면서 편리했던 점과 불편했던 점이 궁금합니다.

Copy link

Choose a reason for hiding this comment

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

아무래도 기본적인 컴포넌트의 디자인과 기능이 전부 구현이 되어 있어서 컴포넌트에 대한 기능 구현이 최소화 된 부분이 가장 편리했던 것 같습니다! 불편했던 점은 기본적인 스타일링이 되어 있기 때문에 프로젝트에 맞게 커스텀 하는 과정들과 style 코드를 추가적으로 작성해줘야 하는 부분들이 조금 불편했던 것 같습니다!

type NotificationPopupProps = {
datas: INotificationData[]
}
export const NotificationPopup = React.memo(({ datas }: NotificationPopupProps) => {

Choose a reason for hiding this comment

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

최적화를 위해 React.memo를 사용한 부분이 인상깊었습니다.

Comment on lines +1 to +4
export * from 'components/common/NavLayout'
export * from 'components/common/AppNav'
export * from 'components/common/NotificationPopup'
export * from 'components/common/SkeletonTable'

Choose a reason for hiding this comment

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

calendar, common, dayoff 등 index.ts 파일을 생성하여 export * from 구문을 사용하여 해당 모듈의 모든 내용을 내보낸 특별한 이유가 궁금합니다!

Copy link

Choose a reason for hiding this comment

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

Named exports를 사용하면 한 파일에서 여러 값을 export하는데 좋습니다! 또한 같은 계층에 있는 모듈을 import 할 때 한줄로 import가 가능합니다. Export default의 경우 import할 모든 모듈을 명시해야하기 때문에 import 구문이 길어지기도 하구요.. 그래서 각 폴더의 루트에 있는 index.ts를 사용하면 하위 파일에서 내보내고 있는 모듈들을 한번에 내보낼 수 있어서 좋다고 생각해서 이런 방식으로 진행하게 되었습니다!

Copy link

@azure0929 azure0929 left a comment

Choose a reason for hiding this comment

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

로그인과 회원가입 부분 고생 많으셨습니다!
인상깊었던 부분 위주로 코멘트 남깁니다!

Comment on lines +19 to +32
const [loginData, setLoginData] = useState<ILoginData>({ email: '', password: '' })
const [emailError, setEmailError] = useState<string>('')
const navigate = useNavigate()
// 로그인한 유저 정보 전역 State
const { setUserInfo } = useUserStore()
const { openModal } = modalStore()

const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const { name, value } = event.target
setLoginData({
...loginData,
[name]: value
})
}

Choose a reason for hiding this comment

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

전역 상태 관리를 위한 useUserSotore 훅과 modalStore 훅을 가져오고, 로그인 데이터의 해당 필드만 업데이트하여 기존 데이터와 병함한 부분이 인상깊었습니다!

Comment on lines +35 to +102
useEffect(() => {
console.log(signUpData)
}, [signUpData])

const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const { name, value } = event.target
setSignUpData({
...signUpData,
[name]: value
})
}

const handleDatePickerChange = (date: Dayjs | null, dateString: string) => {
setDate(date)
setSignUpData({
...signUpData,
joinDate: date ? dateString : ''
})
}

const handlePositionChange = (value: string) => {
setSignUpData({
...signUpData,
position: value
})
}

const handlePasswordConfirmChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const { value } = event.target
setPasswordConfirm(value)
}

const handleSubmit = async () => {
//이메일 유효성검사
const emailRegex = /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$/
if (!emailRegex.test(signUpData.email)) {
setEmailError('유효한 이메일 주소를 입력하세요.')
return
}
//비밀번호확인 유효성검사
if (signUpData.password === passwordConfirm) {
console.log(signUpData)
signUpRequest(signUpData).then(
res => {
console.log('API 호출 성공!')
console.log(res)
openModal({
...resultModalDatas.SIGNUP_SUCCESS,
okCallback: () => {
navigate('/login')
}
})
},
error => {
console.log(error)
openModal({
...resultModalDatas.SIGNUP_FAILURE,
content: error.message || resultModalDatas.SIGNUP_FAILURE.content
})
console.error('API 호출 실패!')
console.error(error)
}
)
} else {
console.log('비밀번호 확인 실패!')
setPasswordMismatch(true)
}
}

Choose a reason for hiding this comment

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

각 부분의 변경 시 핸들러 함수를 정의한 부분이 가장 인상깊었고, useEffect를 사용하여 signupData가 변경될 때마다 로그를 출력하도록 하셨는데 이 부분 잘하셨습니다.

Copy link
Member

@GyoHeon GyoHeon left a comment

Choose a reason for hiding this comment

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

총평

프로젝트의 폴더 구조가 아주 좋습니다.
많은 수강생 분들이 협업시에 자신의 스코프에만 집중해서 코드가 제각각이고 불필요한 재사용이 많은데 6조 분들의 과제는 그런것이 전혀 없습니다.

따라서 특별히 아쉬운 점은 없습니다.
6조 분들의 리뷰는 크게 아쉬운 점이 없어서 괜히 딴지 걸었다 생각하고 맘 편히 보세요.

마무리...

이번 과정에서 제가 여러분께 드리는 마지막 코드리뷰고, 여러분이 받는 마지막 코드리뷰겠네요.
제가 한 모든 코드리뷰는 한 사람의 의견으로 듣되, 왜 이런 의견이 나왔는지 고심해 보시길 바랍니다.
짧은 시간 동안 제가 여러분들의 사수 역할을 조금이나마 해냈다면 좋겠네요.

마지막 프로젝트는 아니지만 그 동안 고생하셨고, 여러분이 노력하신 만큼 결과가 뒤따라오길 기원하겠습니다.
다음에 뵐때는 멘토와 수강생이 아니라 개발자와 개발자로 뵙겠습니다.

수고하셨습니다.

Comment on lines +220 to +225
<Option value="0">사원</Option>
<Option value="1">주임</Option>
<Option value="2">대리</Option>
<Option value="3">과장</Option>
<Option value="4">차장</Option>
<Option value="5">부장</Option>
Copy link
Member

Choose a reason for hiding this comment

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

하드 코딩하지 않고 반복문을 사용하는게 더 좋을 것 같습니다.

Comment on lines +12 to +27
switch (position) {
case 0:
return '사원'
case 1:
return '주임'
case 2:
return '대리'
case 3:
return '과장'
case 4:
return '차장'
case 5:
return '부장'
default:
return ''
}
Copy link
Member

Choose a reason for hiding this comment

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

  1. object로 처리하면 굳이 switch문을 사용하지 않아도 됩니다.
Suggested change
switch (position) {
case 0:
return '사원'
case 1:
return '주임'
case 2:
return '대리'
case 3:
return '과장'
case 4:
return '차장'
case 5:
return '부장'
default:
return ''
}
{
'0': '사원',
'1': '주임',
'2': '대리',
'3': '과장',
'4': '차장',
'5': '부장',
}

Copy link
Member

Choose a reason for hiding this comment

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

그리고 position의 존재 자체가 애매합니다.
굳이 position을 숫자로 처리하지 않고 그냥 '사원' 같이 표현하면 이렇게 parsing할 필요도 없고 바로 사용할 수 있는데다가 position이 3이라고 하면 무슨 직급인지 이해가 가지 않지만 position이 '과장'이라고 하면 이해도 쉽습니다.

Comment on lines +46 to +62
const dateCellRender = (value: Dayjs) => {
const listData =
filteredSchedules.find(
schedule => dayjs(schedule.date).format('YYYY-MM-DD') === value.format('YYYY-MM-DD')
)?.schedules ?? []
return (
<EventUl>
{listData.map((item, index) => (
<li key={index}>
<Tag color={item.color as TagProps['color']}>
{CALENDER_TYPE[item.type]} {item.username} {USER_POSITION[item.position]}
</Tag>
</li>
))}
</EventUl>
)
}
Copy link
Member

Choose a reason for hiding this comment

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

컴포넌트 안에서는 컴포넌트를 선언하면 안됩니다.

참고: https://react.dev/learn/your-first-component#nesting-and-organizing-components

return (
<StyledCalendar
style={{ padding: 20, borderRadius: 6 }}
mode={'month'}
Copy link
Member

Choose a reason for hiding this comment

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

단순 string은 중괄호를 사용하지 않아도됩니다.

import { Card, Badge } from 'antd'
import { styled } from 'styled-components'

export const ScheduleCard = React.memo(({ schedule }: { schedule: ICalendarSchedule }) => {
Copy link
Member

Choose a reason for hiding this comment

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

React.memo, useCallback, useMemo 모두 좋은 메모이제이션입니다.
하지만 메모이제이션 기법을 사용할 때도 생각해야 할 부분이 있습니다.

메모이제이션의 장점은 불필요한 렌더링을 줄이고 이를 통해 리렌더링의 횟수를 줄이는 것입니다.

이때 모든 메모이제이션은 추가적인 코드를 사용하기 때문에 실제 코드가 증가해 번들의 사이즈가 증가합니다.
또한, 메모이제이션 된 결과는 메모리상에 위치하므로 추가적인 메모리도 할당받습니다.

따라서 메모이제이션의 결과값이 작고 리렌더링이 어차피 자주 일어나는 경우 메모이제이션의 이점은 없고 단점만 부각될 수 있습니다.

따라서, 메모이제이션은 습관적으로 사용하지 않고 리렌더링이 불필요하게 많이 일어나거나 리렌더링시 과도한 메모리가 사용되는 경우에만 적용하는 것이 좋습니다.

Comment on lines +24 to +25
export const DayOffRequestModal = React.memo(
({ availableDays, isModalOpen, onClickOk, onClickCancel }: TDayOffRequestModalProps) => {
Copy link
Member

Choose a reason for hiding this comment

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

지금 이 컴포넌트에서 React.memo에 대한 이야기를 좀 더 이어나가 보겠습니다.

DayOffRequestModal 사용되는 컴포넌트는 DayOff 컴포넌트입니다.

해당 컴포넌트에서 state는 아래와 같이 사용합니다.(4개)

  const [isModalOpen, setIsModalOpen] = useState(false)
  const [dayOffList, setDayOffList] = useState<IDayOffResponse[]>([])
  const [isLoading, setIsLoading] = useState(false)
  const [numOfDayOff, setNumOfDayOff] = useState<number>(0)

또, DayOffRequestModal이 받는 props는 아래와 같습니다.

        availableDays={numOfAvailableRequestDays}
        isModalOpen={isModalOpen}
        onClickOk={handleOk}
        onClickCancel={handleCancel}

단순 state를 isModalOpen이 사용하고 있고 numOfAvailableRequestDays라는 함수는 아래와 같습니다.

  const numOfAvailableRequestDays = useMemo(
    () => numOfDayOff - calcNumOfRequest(requestList) - numOfUsedDays,
    [numOfDayOff, requestList, numOfUsedDays]
  )

위 함수는 numOfDayOff라는 state와 requestList, numOfUsedDays에 따라 바뀝니다.
requestList, numOfUsedDays는 아래와 같습니다.

  const requestList = useMemo(() => getFilteredDayOffRequestList(dayOffList), [dayOffList])
  const historyList = useMemo(() => getFilteredDayOffHistoryList(dayOffList), [dayOffList])
  const numOfUsedDays = useMemo(() => calcNumOfUsedDayOff(historyList), [historyList])

requestList는 dayOffList라는 스테이트를 받고 numOfUsedDays -> dayOffList를 받습니다.

따라서 DayOffRequestModal은 isModalOpen, numOfDayOff, dayOffList에 따라 리렌더링 됩니다.

그리고 결정적으로...handleOk 같은 일반 함수는 부모 컴포넌트의 리렌더링 시마다 새로운 함수 참조를 가지므로 부모 컴포넌트의 리렌더링시에 무조건 DayOffRequestModal이 리렌더링 되게 됩니다.

Copy link
Member

Choose a reason for hiding this comment

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

메모이제이션이 잘못 되었고 지양해야 한다는 것은 아닙니다!
하지만, 메모이제이션을 습관처럼 사용하는 것은 좋지 않습니다.
메모이제이션은 꼭 필요한 상황에서만 사용하는 것을 목표로 사용해보세요!

Comment on lines +28 to +30
alert('로그인 세션이 만료되었습니다. 다시 로그인해주세요.')
location.replace('/login')
logout()
Copy link
Member

Choose a reason for hiding this comment

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

alert 이전에 logout 처리를 해야 할 것 같습니다.
alert는 script를 block 합니다.

Comment on lines +1 to +12
export const colorOfType = (type: number) => {
switch (type) {
case 0:
return 'geekblue'
case 1:
return 'cyan'
case 2:
return 'green'
default:
return 'orange'
}
}
Copy link
Member

Choose a reason for hiding this comment

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

object로 처리할 수도 있겠습니다.(default는 else로...)

Suggested change
export const colorOfType = (type: number) => {
switch (type) {
case 0:
return 'geekblue'
case 1:
return 'cyan'
case 2:
return 'green'
default:
return 'orange'
}
}
export const colorOfType = {
'0': 'geekblue',
'1': 'cyan',
'2': 'green',
}

Copy link
Member

Choose a reason for hiding this comment

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

혹은 그냥 type을 색으로 지정해두면 굳이 parsing하지 않을 수 있습니다.

Comment on lines +12 to +31
case 0:
if (dayjs().startOf('date').diff(data.startDate) < 0) {
filteredList.push(data)
}
break
case 1:
if (
dayjs().startOf('date').diff(data.startDate) < 0 ||
(dayjs().startOf('date').diff(data.startDate) === 0 && dayjs().get('hour') < 9)
) {
filteredList.push(data)
}
break
case 2:
if (
dayjs().startOf('date').diff(data.startDate) < 0 ||
(dayjs().startOf('date').diff(data.startDate) === 0 && dayjs().get('hour') < 14)
) {
filteredList.push(data)
}
Copy link
Member

Choose a reason for hiding this comment

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

if 문 안의 조건문이 반복되는군요.
변수로 처리하면 반복을 줄일 수 있겠습니다.

Comment on lines +93 to +110
dayoffList.map(dayoff => {
if (dayoff.startDate !== dayoff.endDate) {
let date = dayjs(dayoff.startDate)
const diffDays = dayjs(dayoff.endDate).diff(date, 'day')
for (let i = 0; i <= diffDays; i++) {
if (date.get('day') !== 0 && date.get('day') !== 6) {
dayOffSchedules.push({
...dayoff,
color: colorOfType(dayoff.type),
startDate: date.format('YYYY-MM-DD')
})
}
date = date.add(1, 'day')
}
} else {
dayOffSchedules.push({ ...dayoff, color: colorOfType(dayoff.type) })
}
})
Copy link
Member

Choose a reason for hiding this comment

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

map을 돌리면서 변형된 데이터를 dayOffSchedules에 넣어주고 있습니다.
다음과 같은 문제가 있습니다.

  1. map은 배열을 리턴하는 고차함수입니다. 하지만 해당 map의 리턴값을 전혀 사용하지 않고 있습니다. 이 경우엔 map이 아니라 forEach를 사용하는 것이 더 적절한 선택입니다.
  2. map은 새로운 배열을 리턴하므로 map의 리턴을 바로 사용할 수 있습니다. 즉, dayOffSchedules을 빈 배열로 생성 -> dayoffList 순회 로직 -> dayOffSchedules 리턴의 과정을 dayoffList 순회 로직 및 리턴 으로 줄일 수 있습니다.

Copy link

Choose a reason for hiding this comment

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

감사합니다. 멘토님!
코멘트로 달아주신 부분들 꼼꼼하게 다시 보면서 리팩토링 해보도록 하겠습니다.
자세한 코드리뷰 감사드립니다. 비록 짧은 시간이지만 그동안 많은 조언과 격려 해주셔서 감사합니다 :)

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.

4 participants