영광스럽게도 포코와 헤인티의 리뷰를 둘 다 받을 수 있었어요. 엄청났습니다. 100개가 넘는 리뷰가 오고갔거든요!!
리뷰 반영하다가 울었습니다
엄청 두드려 맞았어요ㅋㅋ 근데 코드를 수정하면서 맞을만 했다고 생각합니다.
다음 미션에 바로 반영할 수 있을 정도로 많은 연습이 되었다고 생각해요. 오히려 좋아!!
💪 Javascript
렌더링은 최소화
const template = videoStorage.getVideoDataList().map(videoData => {
return `템플릿만들기`
})
// 한 번만 DOM을 만들어줍시다!
this.$storedVideoList.insertAdjacentHTML('beforeend', template.join(''));- innerHTML, insertAdjacentHTML 같은 작업은 비용이 많이 들어요.
반복문으로 template을 전부 만들고 나서 innerHTML, insertAdjacentHTML 해주는 작업은 한 번만 할 수 있도록 합시다.
DOM에 의존하지 말자
- 사실 전에도 받았던 피드백이죠?
최대한 element에 id를 부여해서 가져오도록 합시다.
변수로 구체화하자
this.renderSwitchedVideoData(storedList[storedList.length - 1]);- 위 코드에서
storedList.length - 1를 왜하는지 알 수 있나요?
제 3자가 읽어도 문제가 없을 정도의 구체화를 해줍시다.
예시 1)
// 최소한 이렇게요!
const switchedVideo = storedList.length-1;
// 적어도 바뀐 비디오를 재렌더링한다고 느낌이 오겠죠?예시 2)
if(this.$videoList.scrollHeight - this.$videoList.scrollTop <= this.$videoList.offsetHeight + EVENT.SCROLL.OFFSET
) {
// 무한 스크롤...
}
// 위보다는 이렇게!
const isScrollBottom = this.$videoList.scrollHeight - this.$videoList.scrollTop <= this.$videoList.offsetHeight + EVENT.SCROLL.OFFSET;
if(isScrollBottom) {
// 무한 스크롤...
}네이밍도 도메인과 UI를 분리하자
parseClickedData(clickedVideo)
vs
parseVideoInfo(videoInfo)- 이 메서드는 클릭한 비디오의 정보를 원하는 형태로 파싱하는 메서드예요.
과연 이 메서드 네이밍에 Click이라는 유저 행위가 포함되는 것이 맞을까요? 고민해봅시다!
구조분해 할당을 적극 활용하자
const videoId = clickedVideo.children[4].dataset.videoid;
const publishedAt = clickedVideo.children[3].textContent;
const title = clickedVideo.children[1].textContent;
const url = clickedVideo.children[0].src;
const channelTitle = clickedVideo.children[2].textContent;
vs
const [thumbnailImg, title, channelTitle, publishedAt, videoId] = videoInfo.children;- 확실히 보기 편하고 코드길이도 줄어들죠?
만능 플래그는 위험해
// showType이라는 플래그는 막 변할 수 있쬬? 매우 위험한 상태
export default class MainView {
constructor() {
this.registerDOM();
this.renderStoredVideoList();
this.showType = VIDEO_TYPE.WATCH_LATER;
----------------------------vs----------------------------
this.#showType = VIDEO_TYPE.WATCH_LATER;
get showType(type) {
return tihs.#showType
}
set showType(type) { // type은 'watch-later', 'watched' 값이 들어올 것으로 기대합니다.
if (this.#showType === type) { // 만약 타입이 그대로라면 변경 x
return;
}
if (type !== VIDEO_TYPE.WATCH_LATER && type !== VIDEO_TYPE.WATCHED) { // 'watch-later', 'watched' 둘 다 아니면 변경 x
return;
}
this.#showType = type;
}- showType을 private으로 바꿔주고 get과 set으로만 접근할 수 있도록 해줍시다.
추가적으로 Typescript에서 showType을 위한 타입을 지정해주면 더 안전할 것 같다는 생각이 드네요.
생각보다 Array 내장 메서드는 유용하다
getDataIndexInList(videoId) {
let dataIndex = 0;
this.getVideoDataList().forEach((videoData, idx) => {
...
}
}
----------------------------vs----------------------------
getDataIndexInList(videoId) {
let dataIndex = 0;
this.getVideoDataList().findIndex((videoData) => {
...
}
}- 무지성 forEach 멈춰!
findIndex라는 좋은 메서드가 있네요. forEach를 생각하기 이전에 문서를 참고해서 만들어봅시다!
Array 내장 메서드 MDN
mock Data는 변경 금지
파싱하는 과정이 있어서 임의대로 mock Data를 변경해줬어요.
publishedAt: '2021-10-14T19:06:42Z',
// 위의 내용을 아래처럼 변경해줬어요.
publishedAt: '2021년 10월 14일',하지만 혼났습니다.
기획자가 목데이터 던져줄때마다 한땀한땀 바꾸실건가용?
바로 인정... 바꾸지 맙시다!
상수 네이밍은 언제나 대문자
- 가끔씩 실수를 하는데 mockData도 상수입니다. MOCK_DATA가 맞겠죠?
toggle을 이용해보자
if (checkRendering) {
this.$emptyListImage.classList.remove(DOM_STRING.HIDE);
return;
}
this.$emptyListImage.classList.add(DOM_STRING.HIDE);
----------------------------vs----------------------------
this.$emptyListImage.classList.toggle(query, condition);
- toggle을 통하면 편하고 코드도 줄어들죠?
하지만 언제나 toggle을 쓰는건 별로라고 느꼈어요. (예를 들면 snackBar를 사용할 때... 반복적으로 snackBar가 호출되면 쓸데없이 계속 toggle이 되더라구요.)
classList도 좋은 메서드가 많다
if([...event.target.classList].includes(DOM_STRING.VIDEO_ITEM_SAVE_BUTTON))
----------------------------vs----------------------------
if(event.target.classList.contains(DOM_STRING.VIDEO_ITEM_SAVE_BUTTON))- 사고를 넓혀요! Array 내장 메서드만 생각하지말아요. 웬만해선 다~ 있어요.
공식문서를 읽는 습관을 들입시다.
localStorage 접근은 생각보다 비싼 작업이다
localStorage.getItem은 최소한으로 불릴 수 있도록 작업합시다.
함께 웃어야 행복한 개발자 장호영입니다😃