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

[김수경] sprint5 #126

Conversation

suKyoung25
Copy link
Collaborator

멘토에게

  1. 파일이 중간에 날아가서 코드가 많이 뒤섞여있습니다. src/App.js 파일 위주로 봐주시면 좋겠습니다.
  2. npm run dev 커맨드를 실행하면 public/index.html을 실행하여 브라우저를 띄운다고 알고있습니다. 그 외 다른 파일이나 폴더에는 html이 존재하지 않고요. 그런데 제 코드는 index.html이 public 외부에 있어야만 브라우저를 렌더링하는데 뭐가 문제일까요?

@suKyoung25 suKyoung25 requested a review from AlskaPark March 7, 2025 15:09
@suKyoung25 suKyoung25 changed the title React 김수경 sprint5 [김수경] sprint5 Mar 7, 2025
Copy link
Collaborator

@AlskaPark AlskaPark left a comment

Choose a reason for hiding this comment

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

로직은 괜찮습니다. 다만 테스트 코드와 더미 코드가 많고 불필요한 파일들이 보입니다. 코드와 파일 정리가 필요할거 같습니다.



// GET 메서드를 사용하여 기사 목록을 가져오는 함수
function getArticleList(page = 1, pageSize = 10, keyword = '???') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

기본값으로는 ''(빈 문자열) 또는 null을 사용하는 것이 좋습니다. 일반적으로 코드에서 비어있는 값을 따로 처리하는 관례가 있기 때문입니다.

return response.json();
})
.then(data => {
console.log(data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

포맷팅은 작업 후 한번씩 해주시면 좋습니다. 실무에서는 어쩌면 사소하지만 채용시장에서 상당히 중요하게 작용할 수 있습니다!

function getArticleList(page = 1, pageSize = 10, keyword = '???') {
const url = `https://panda-market-api-crud.vercel.app/articles/?page=${page}&pageSize=${pageSize}&keyword=${keyword}`;

fetch(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

아마 console.log로 작동을 확인해서 return을 안 하신거 같습니다..ㅎㅎ return을 해주시고 함수 밖에서 console.log로 확인해보시길 권장합니다. 왜냐하면 내부 console.log는 해당 함수와는 관련이 없는 테스트 코드이기 때문입니다.

@@ -0,0 +1,3 @@
[ZoneTransfer]
Copy link
Collaborator

Choose a reason for hiding this comment

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

.identifier.gitignore에 등록해주시면 좋습니다 :)

function getBestPageSize() {
const width = window.innerWidth;
if (width >= 1200) {
return 4; // pc화면
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 값을 magic number라고 하는데요. 상수로 빼서 이름을 붙여주면 좋습니다~!

const BASE_URL = "https://panda-market-api.vercel.app";

//Get 메서드를 이용하여 제품 목록을 가져오는 함수
async function GetProductList(options) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

export 함수의 시작을 대분자로 하는 언어도 있는데요. JavaScript 진영에서는 권장하지 않는 방법입니다. getProductList와 같이 camelCase를 사용하시는걸 권장드려요.

@AlskaPark AlskaPark merged commit 9904fff into codeit-sprint-fullstack:react-김수경 Mar 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants