eliotjang / Top-Rated-Movie-List-Website

Use TMDB Open API
0 stars 0 forks source link

feedback #1

Open ggomma opened 2 months ago

ggomma commented 2 months ago

구현 사항 확인

필수

선택

피드백

총평

개선점

파일 분리

현재 html 파일 안에 css, js 코드가 모두 합쳐져 있습니다. 이 경우 서비스가 복잡해지면 코드를 이해하기 어려워집니다. 이를 방지하기 위해서는 css, js파일을 별도로 분리하여 html 파일에서 불러오는 것이 좋습니다.

검색 버그

존재하지 않는 영화에 대한 검색을 수행하면 화면에 아무것도 노출되지 않습니다. 이는 정상 작동이지만 이후 다른 검색을 수행하여도 화면에 아무것도 노출되고 있지 않습니다. 이를 해결하기 위해서는 아래 코드를 수정하시면 됩니다.

https://github.com/eliotjang/Top-Rated-Movie-List-Website/blob/18062d378170ce72ed8607b1248792660018e680/main.js#L101-L111

let 변수

https://github.com/eliotjang/Top-Rated-Movie-List-Website/blob/18062d378170ce72ed8607b1248792660018e680/main.js#L102

let은 변수의 값이 변화하는 경우 사용합니다. 현재 코드에서는 value, cardData의 값이 변하지 않기에 const 를 사용하는 것을 추천합니다.

https://github.com/eliotjang/Top-Rated-Movie-List-Website/blob/18062d378170ce72ed8607b1248792660018e680/main.js#L105

추가로 for 구문에서는 일반적으로 for (let i=0; i < cardData.length; i++) 와 같은 방식으로 i를 선언합니다.

변수 선언

javascript에서는 변수를 선언할 때 camelCase라는 방식을 사용합니다. 관련한 내용은 여기를 참고해보세요.

추가 요소

아래 추가 개발을 권장하는 내용들입니다.

eliotjang commented 2 months ago

검색 버그 이슈 해결

107 Line의 movieName[0].innerHTML.toUpperCase().indexOf(value) > -1 조건이 아닌 경우에 110 Line에서 cardData[i].style.display = "none"; 을 추가하여 처음 검색 이후 다음 검색에 노출이 안되는 버그 해결

let 변수 이슈 해결

101 Line의 value, cardData를 const로 지정하고 i는 for 반복문 안에서 초기화 설정하여 let 변수 이슈 해결

변수 선언 이슈 해결

파일 분리 이슈 문의

현재 index.html 파일 외부에 main.js 파일과 style.css 파일로 별도 분리되어있습니다. Feedback 사항이 assets, src, style 폴더 등으로 구분한 이후에 main.js, movie.js, search.sj 등으로 분리하라는 말씀이신건지 문의드립니다.

화면 캡처 2024-04-29 193159

@ggomma