프로젝트 피드백 - 예병수 튜터

DevelopSoo commented 2 months ago

피드백

추가 피드백 constants

api.js

getTopRatedMovies 에서 에러 발생 시 alert로 에러 객체를 보여줍니다. 사용자는 개발자 데이터를 보고 이해할 수 없으니 좀 더 친절한 설명을 추가하시는 것이 좋겠습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/api.js#L15-L17
getMovieDetails 함수에서 오류 발생 시 메인 페이지로 리디렉션하는 로직이 있습니다. 이는 함수의 예상 동작과 다를 수 있으므로, 리디렉션은 함수 외부에서 처리하는 것이 좋습니다. 함수 이름에서 외부로 이어지는 동작은 예측할 수 없을 수 있기 때문입니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/api.js#L31
getMovieTrailer 함수에서 언어 설정이 en-EN으로 되어 있습니다. 이는 표준 언어 코드가 아닙니다 (en-US가 맞을 것입니다). 문서를 확인해보니 기본이 en-US로 되어있는 것 같습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/api.js#L53

detail.js

getMovieDetails() 함수가 두 번 호출되고 있습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/details.js#L57-L58
모달을 닫는 로직이 중복되어 있습니다. modalImg.onclick과 window.onclick에서 모달을 닫는 코드가 반복되고 있습니다. 함수로 분리하여 중복을 줄일 수 있습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/details.js#L22-L29
```
function closeModal() {
  modal.style.display = "none";
}

modalImg.onclick = closeModal;
window.onclick = (event) => {
  if (event.target == modal) {
    closeModal();
  }
};
```

index.js

현재 코드에서는 외부 API 호출(get20Movies) 시 발생할 수 있는 에러에 대한 처리가 보이지 않습니다. 에러 처리 로직을 추가하여 사용자에게 적절한 피드백을 제공하는 것이 좋습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/index.js#L30

comment.js → 사용하지 않는 파일은 삭제해주세요

event.js

hideScrollTopButtonOnTop 함수가 사용되는 곳을 보니 스크롤 이벤트에 연결되어 있네요. 스크롤 이벤트는 매우 빈번하게 발생하므로, 성능을 고려하여 이throttle 또는 debounce 기법을 사용하여 이벤트 핸들러의 호출 빈도를 제어하는 방법을 찾아보시면 좋습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/feature/event.js#L13-L17

genre.js

"" + genre.id는 문자열로 변환하려고 하신건가요? String(genre.id)를 사용하는 것이 더 명시적입니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/feature/genre.js#L11

search.js

showCardsByQuery 함수는 카드를 보여주는 것 뿐만 아니라 일치하는 카드의 수를 반환합니다. 이 두 기능을 분리하거나 함수 이름을 변경하여 기능을 명확히 할 수 있습니다. https://github.com/2hanbyeol1/movie-movie/blob/7f9170bdda4f5d2d2513de8fe463c40d6621dacb/src/feature/search.js#L11-L21

validation.js

2hanbyeol1 commented 2 months ago

엄청난 피드백 감사합니다 ☺️

phantom2115 commented 2 months ago

감사합니다 튜터님

2hanbyeol1 / movie-movie