[클린코드 8기 유승완] 영화 리뷰 미션 STEP 1 #43

Seung-wan · 2025-09-17T18:15:14Z

안녕하세요!

이번주 목/금이 회사 플레이샵이라서 최대한 작업해보고 먼저 PR 올려봅니다!
main.js에서 다양한 책임을 다루고 있어서 이걸 잘 분리하는 작업을 차차 해가야 될 것 같아요!

…eleton UI를 구현한다.

Seung-wan · 2025-09-17T18:17:39Z

src/apis/getPopularMovies.js

+  const url = new URL("https://api.themoviedb.org/3/movie/popular");
+  url.searchParams.set("language", language);
+  url.searchParams.set("page", page);
+
+  const response = await fetch(url.toString(), {
+    headers: {
+      accept: "application/json",
+      Authorization: `Bearer ${import.meta.env.VITE_TMDB_API_KEY}`,
+    },
+  });


현재는 api server url을 환경변수로 다룰 필요가 없을 것 같아서 그렇게 다루진 않았고, api 함수가 딱 하나라서 fetch와 관련해서도 별도 추상화를 하지는 않았어요!

좋습니다. 그런데 헤더 이미지를 변경하시게 된다면 new URL에 선언하신 하드 코딩된 날것의 URL주소는 별도의 상수로 선언하고 관리하시는 게 편하실지도 모르겠어요 ㅎ

Seung-wan · 2025-09-17T18:18:54Z

src/apis/getPopularMovies.js

+  };
+}
+
+function toClientEntity(popularMovies) {


외부로 export 되지 않고 모듈에서만 사용되는 함수명은 너무 이상하지만 않다면 합의만 되면 괜찮다고 생각해서 요정도로 지어봤어요.
파생값을 만들어주기 위한 역할을 할 수도 있지만, 현재는 snake_case를 camelCase로 변환하는 역할을 담당하고 있어요.

네넵~! 아무래도 미션에서 사용하는 컨벤션을 준수하시려는 의도로 보였습니다. 어떤 방향이든 좋아요!

Seung-wan · 2025-09-17T18:20:06Z

src/components/MovieCard.js

+  return li;
+}
+
+export function createMovieCardSkeleton() {


리액트에서는 MovieCard 라는 컴포넌트가 있다고 하면, MovieCard.Skeleton = MovieCardSkeleton과 같이 표현하는 걸 좋아하는데 여기서는 스켈레톤 관련된 코드를 MovieCard라는 파일안에 같이 두는 방식으로 작업했어요.

Seung-wan · 2025-09-17T18:20:54Z

src/domain/formatMovieRate.js

+export function formatMovieRate(rate) {
+  return rate.toFixed(1);
+}


이 함수는 동작 자체는 단순하지만, 영화 서비스에서 평점은 다양한 곳에서 표현될 것으로 예상이 되어서 별도 함수로 분리해두었어요

Seung-wan · 2025-09-17T18:22:20Z

src/domain/getImageUrl.js

+export function getImageUrl(posterPath) {
+  return `https://media.themoviedb.org/t/p/w440_and_h660_face/${posterPath}`;
+}


요건 네이밍을 조금 더 좁히면 좋겠다는 생각이 들어요.
현재는 크게 상관은 없지만, 조금만 요구사항이 추가되어도 바로 재사용이 불가능 할 것 같아요.

tmdb에서 제공하는 스펙을 봐야겠지만 width랑 height를 받을 수 있다면 받아보면 좋을 것 같네요. + face같이 이미지 타입을 결정하는 값도 같이? (face가 아직 뭔지는 모르겠어요)

Seung-wan · 2025-09-17T18:22:50Z

src/main.js

+  const wrap = document.createElement("div");
+  wrap.id = "wrap";
+  app.append(wrap);
+
+  const header = document.createElement("header");
+  wrap.append(header);
+
+  const banner = createBanner();
+  header.append(banner);
+
+  const container = document.createElement("div");
+  container.classList.add("container");
+  wrap.append(container);
+
+  const main = document.createElement("main");
+  container.append(main);
+
+  const section = document.createElement("section");
+  main.append(section);
+
+  const title = document.createElement("h2");
+  title.textContent = "지금 인기 있는 영화";
+  section.append(title);


요렇게 기본 템플릿을 만들어주는 부분들 코드가 조금 불편한 것 같아요.
특정 단위로 묶어야 될 것 같다는 생각이 들었어요

Seung-wan · 2025-09-17T18:23:37Z

src/main.js

+  const loadMoreButton = document.createElement("button");
+  loadMoreButton.textContent = "더 보기";
+  loadMoreButton.classList.add("load-more-button");
+  loadMoreButton.addEventListener("click", async () => {
+    const cardSkeleton = Array.from({ length: ITEM_PER_PAGE }, () =>
+      createMovieCardSkeleton()
+    );
+    ul.append(...cardSkeleton);
+
+    page = page + 1;
+    const response = await getPopularMovies({
+      language: "ko-KR",
+      page,
+    });
+
+    if (response.total_pages === page) {
+      loadMoreButton.classList.add("hide");
+    }
+
+    const cards = response.results.map(({ title, posterPath, voteAverage }) =>
+      createMovieCard({ title, imageFileName: posterPath, rate: voteAverage })
+    );
+
+    ul.querySelectorAll(".card-skeleton").forEach((el) => el.remove());
+
+    ul.append(...cards);
+  });
+


이 부분이 현재 하는일이 되게 많으면서 추상화 수준이 낮아서 이해하기 되게 어려운데, 여기 loadMoreButton은 이전 수업에서 보여주셨던 것처럼 객체로 관리해봐야겠다 싶었어요.

sannimdev

승완님! 제일 먼저 제출해 주셨네요!
미션에 열정적으로 참여해주셔서 감사합니다 🙏🏻

리뷰 남겨 드렸으니 코멘트를 참고해 주세요!
그리고 다음 요청에는 배포 URL (Github Pages)도 챙겨주시면 감사하겠습니다 🙂

사용성

배포URL이 없어서 로컬에서 저의 API_KEY를 넣고 구동해 봤는데 더 보기 버튼을 누르기 전 페이지 로드할 때에도 Skeleton이 보이지 않습니다.
더 보기 버튼을 누르면 Skeleton이 로드되고 네트워크로 서버에 다음 페이지 요청은 이루어지고 있으나, 로드된 콘텐츠가 보이지 않습니다.
다음 페이지 로드 후 Skeleton 대신 원래 콘텐츠가 렌더링되지 않아 자세히는 모르지만, 페이지가 다 불러와지기 전에 더 보기 버튼은 숨기거나 눌리지 않도록 해야 하지 않을까요? 사용자는 그 다음 페이지를 불러오는 걸 원하지 않을 수도 있거나 또는 갑작스러울 수 있으니까요🙂.

sannimdev · 2025-09-19T05:17:26Z

src/apis/getPopularMovies.js

영화 리뷰 헤더 영역의 배너 포스터 이미지는 아래의 링크로 불러와지도록 하면 더욱 멋질 것 같아요!

const 배너URL = "https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/";

sannimdev · 2025-09-19T05:19:00Z

src/apis/getPopularMovies.js

+  };
+}
+
+function toClientEntity(popularMovies) {


네넵~! 아무래도 미션에서 사용하는 컨벤션을 준수하시려는 의도로 보였습니다. 어떤 방향이든 좋아요!

sannimdev · 2025-09-19T05:40:36Z

src/main.js

+  const wrap = document.createElement("div");
+  wrap.id = "wrap";
+  app.append(wrap);
+
+  const header = document.createElement("header");
+  wrap.append(header);
+
+  const banner = createBanner();
+  header.append(banner);
+
+  const container = document.createElement("div");
+  container.classList.add("container");
+  wrap.append(container);
+
+  const main = document.createElement("main");
+  container.append(main);
+
+  const section = document.createElement("section");
+  main.append(section);
+
+  const title = document.createElement("h2");
+  title.textContent = "지금 인기 있는 영화";
+  section.append(title);


아래와 같이 선언부와 DOM 조작부를 따로 나누어서 어떤 요소를 선언했는지 명확하게 알면 더 좋지 않을까요?

Suggested change

const wrap = document.createElement("div");

wrap.id = "wrap";

app.append(wrap);

const header = document.createElement("header");

wrap.append(header);

const banner = createBanner();

header.append(banner);

const container = document.createElement("div");

container.classList.add("container");

wrap.append(container);

const main = document.createElement("main");

container.append(main);

const section = document.createElement("section");

main.append(section);

const title = document.createElement("h2");

title.textContent = "지금 인기 있는 영화";

section.append(title);

const wrap = document.createElement("div");

const header = document.createElement("header");

const container = document.createElement("div");

const main = document.createElement("main");

const section = document.createElement("section");

const title = document.createElement("h2");

wrap.id = "wrap";

app.append(wrap);

wrap.append(header);

header.append(createBanner());

container.classList.add("container");

wrap.append(container);

container.append(main);

main.append(section);

title.textContent = "지금 인기 있는 영화";

section.append(title);

이것마저도 너무 지저분하다는 느낌이 드신다면, 큰 틀을 구성해 주는 함수를 만들어봐도 좋을 것 같아요 :)
예를 들어, createWrap 함수를 만들고 그 안에 header, container, footer를 담는 식으로 말이에요!

function createAppLayout() { const elements = { wrap: document.createElement("div"), header: document.createElement("header"), container: document.createElement("div"), main: document.createElement("main"), section: document.createElement("section") }; // 구조 설정 elements.wrap.id = "wrap"; elements.container.classList.add("container"); // DOM 트리 구성 app.append(elements.wrap); elements.wrap.append(elements.header, elements.container); elements.container.append(elements.main); elements.main.append(elements.section); return elements; }

sannimdev · 2025-09-19T05:43:36Z

src/main.js

+  title.textContent = "지금 인기 있는 영화";
+  section.append(title);
+
+  let page = 1;


이벤트 리스너에 page를 let으로 선언하기보다, 글로벌 스코프에 하나의 객체를 선언하고 앱 내에서 구동될 객체 정보를 정의해 보는 것도 괜찮지 않을까요?

const appState = { page: 1, // 향후 추가될 상태들 // filter: 'all', // sortBy: 'popularity' };

sannimdev · 2025-09-19T05:53:05Z

src/main.js

+    try {
+      const response = await getPopularMovies({
+        language: "ko-KR",
+        page,
+      });
+      if (response.total_pages === page) {
+        loadMoreButton.classList.add("hide");
+      }
+
+      const cards = response.results.map(({ title, posterPath, voteAverage }) =>
+        createMovieCard({ title, imageFileName: posterPath, rate: voteAverage })
+      );
+
+      removeSkeleton();
+      ul.append(...cards);
+    } catch (error) {
+      removeSkeleton();
+      // TODO: Toast나 Alert로 표현하기
+      window.alert(error.message);
+    }


버튼 이벤트리스너는 버튼을 클릭했을 떄 어떤 행동을 할 것인가를 기대하는 것인데, 지금 너무 많은 작업을 하다 보니까 한눈에 보기가 어렵습니다ㅠ.
모든 작업 내역을 담기보다는 별도의 함수로 빼는 작업이 필요하지 않을까요?
예를 들어, 영화 목록을 불러오는 작업의 내역은 별도의 함수로 분리할 수도 있지 않을까요?

async function loadMoreMovies() { try { showLoadingSkeleton(); const newMovies = await fetchNextPage(); hideLoadMoreButtonIfLastPage(newMovies); displayNewMovies(newMovies); } catch (error) { handleLoadError(error); } finally { hideLoadingSkeleton(); } }

현재 리스너가 담당하는 작업을 제가 정리해 봤어요. 어떻게 하면 좀 더 간소화할 수 있을까요?

스켈레톤 UI 작업

페이지 상태 변경

API 호출

마지막 페이지 처리

영화 카드 생성

DOM 조작

에러 처리

sannimdev · 2025-09-19T05:54:11Z

src/main.js

 });
+
+function removeSkeleton() {
+  ul.querySelectorAll(".card-skeleton").forEach((el) => el.remove());


사용성에서 피드백 드렸지만, 더보기 버튼을 누르고 서버에 다음 페이지는 요청하고 있는데, 스켈레톤이 다음 페이지 콘텐츠로 교체되지 않는 부분도 꼭 점검 부탁드려요!

sannimdev · 2025-09-19T05:54:57Z

src/components/Banner.js

+          <img src="./images/star_empty.png" class="star" />
+          <span class="rate-value">9.5</span>
+        </div>
+        <div class="title">인사이드 아웃2</div>


인사이드 아웃2는 더미데이터를 의도해서 넣어두신 걸까용?

sannimdev · 2025-09-19T05:55:39Z

src/apis/getPopularMovies.js

+
+  if (!response.ok) {
+    const error = await response.json();
+    throw new Error(error.status_message || "일시적으로 에러가 발생했어요.");


이 구문에서 어떤 에러가 발생할 수 있을까요?
그리고 어떻게 사용자에게 오류 메시지를 보여주면 좋을까요?

sannimdev · 2025-09-19T05:56:33Z

src/apis/getPopularMovies.js

+  const url = new URL("https://api.themoviedb.org/3/movie/popular");
+  url.searchParams.set("language", language);
+  url.searchParams.set("page", page);


제공되는 객체를 잘 활용하셨네요! 👍

sannimdev · 2025-09-19T06:05:20Z

src/apis/getPopularMovies.js

+  const url = new URL("https://api.themoviedb.org/3/movie/popular");
+  url.searchParams.set("language", language);
+  url.searchParams.set("page", page);
+
+  const response = await fetch(url.toString(), {
+    headers: {
+      accept: "application/json",
+      Authorization: `Bearer ${import.meta.env.VITE_TMDB_API_KEY}`,
+    },
+  });


좋습니다. 그런데 헤더 이미지를 변경하시게 된다면 new URL에 선언하신 하드 코딩된 날것의 URL주소는 별도의 상수로 선언하고 관리하시는 게 편하실지도 모르겠어요 ㅎ

Seung-wan added 9 commits September 17, 2025 02:55

요구사항을 작성합니다.

b1f547a

html 파일에서 css를 불러오기 위해 link 태그를 추가합니다.

71b6a76

배너를 추가합니다.

b492780

영화 목록의 1페이지를 불러오며 더보기 버튼을 누르면 그 다음의 영화 목록을 불러 올 수 있다.영화 목록 아이템에 대한 Sk…

4108d43

…eleton UI를 구현한다.

테스트 코드를 작성합니다.

9c92004

prd를 체크합니다.

587c65e

배포 경로에러 수정

0aee215

배포 경로에러 수정

d7524e8

EOL 제거

78f269c

Seung-wan commented Sep 17, 2025

View reviewed changes

비동기 에러 처리

8434f27

sannimdev requested changes Sep 19, 2025

View reviewed changes

sannimdev self-assigned this Sep 19, 2025

[클린코드 8기 유승완] 영화 리뷰 미션 STEP 1 #43

Are you sure you want to change the base?

[클린코드 8기 유승완] 영화 리뷰 미션 STEP 1 #43

Uh oh!

Conversation

Seung-wan commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sannimdev left a comment

Choose a reason for hiding this comment

사용성

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants