-
Notifications
You must be signed in to change notification settings - Fork 36
[최은영] sprint2 #49
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
The head ref may contain hidden characters: "sprint2-\uCD5C\uC740\uC601"
[최은영] sprint2 #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
은영님, 자신감이 많이 낮아보이는 README 와 달리,
코드에는 은영님이 깊게 생각했다는 점들이 많이 보여서 굉장히 놀란 부분들이 많습니다.
물론, 고쳐야할 부분들도 당연히 있지요
그러나 그건 시간과 노력이 해결해줄거예요
은영님처럼 깊게 생각하는 "능력"은 노력으로 쉽게 얻어지지 않습니다.
그래서 굉장히 인상깊게 코드리뷰를 했습니다.
하나하나 차근차근, 좌절하지 않고 나아가는게 중요합니다!
export function getArticleList(params = { page: 1, pageSize: 10, keyword: '' }) { | ||
|
||
// URL 객체를 생성하여 쿼리 파라미터를 처리 | ||
const url = new URL('${BASE_URL}/articles'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
template literal 을 활용하여 좋은 코드를 작성했습니다 너무좋습니다!
더 나아간다면,
- Base_URL 을 다른 파일로 분리하여, import 했다면 좋았을 것 같습니다
- 변수명을 아예 BASE_URL 이런식으로 했다면 가독성이 더 좋았을 듯 합니다
const Base_URL = 'https://panda-market-api-crud.vercel.app'; | ||
|
||
// 게시글 리스트를 가져오는 함수 (page, pageSize, keyword 쿼리 파라미터를 이용) | ||
export function getArticleList(params = { page: 1, pageSize: 10, keyword: '' }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
params 에 대한 Object 활용 및 디폴트 선언을 한 점 아주 훌륭합니다 !
return fetch(url) | ||
.then(res => { | ||
// 응답이 실패(2XX 범위 밖)할 시, err.massage 출력한 후 err를 throw | ||
if (!res.ok) throw new Error('목록 조회 실패: ${res.status}'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석과 코드가 조금 다른 듯 합니다.
단순히 ok = 2xx 대는 아닙니다!
따라서 header의 httpStatusCode를 활용하여 했다면 더 좋았을 듯 합니다 !
method: 'PATCH', // PATCH 메서드 사용 | ||
headers: { 'Content-Type': 'application/json' }, | ||
body: JSON.stringify({ //수정 내용을 JSON으로 전달 | ||
title, | ||
content, | ||
image, | ||
}), | ||
}) | ||
.then(res => { | ||
if (!res.ok) throw new Error('수정 실패: ${res.status}'); | ||
return res.json(); | ||
}) | ||
.catch(err => console,error('patchArticle 에러:', err.message)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent (들여쓰기) 를 맞추어 가독성을 신경썼다면 더 좋았을 것 같습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스는 이렇게 한 곳에 모으기 보다는 관점에 따라서 분리했다면 더 좋았을 것 같습니다!
products.js
articles.js
와 같이요 !
this.title = title; // 게시글 제목 | ||
this.content = content; // 게시글 내용 | ||
this.writer = writer; // 작성자 이름 | ||
this.likeCount = 0; // 좋아요 수 (초기값 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
비즈니스적 관점에서 초기 생성시에 0 을 지정해주는 것 좋습니다.
너무 잘하셔서 더 나아간 점을 추가로 피드백 드리자면,
초기 생성시에는 0이 맞으나, 나중에 Database에 들어간 값이거나, 어디서 불러온 것이라면 초기에 0이 아닐 수 있으므로, 생성자에 받는것이 오히려 좋을 수 있습니다 !
constructor(name, description, price, tags = [], images = []) { | ||
this.name = name; //상품명 | ||
this.description = description; //상품 설명 | ||
this.price = price; //상품 가격 | ||
this.tags = tags; //상품 해시태그 배열 | ||
this.images = images; //상품 이미지 배열 | ||
this.favorite = 0; //찜하기 수 (초기값 0) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#
문법을 통해, 변수들을 private 화 하여, 캡슐화를 진행해보는건 어땠을까요 ~?
this.#name = name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 indent가 잘 돼있지 않아 코드가 가독성이 떨어지는 것 같습니다!
이부분을 직접 수정하거나,
추후에는 eslint
를 활용해보는것도 좋을 것 같습니다 !
@@ -17,5 +20,5 @@ | |||
 | |||
|
|||
## 멘토에게 | |||
- 셀프 코드 리뷰를 통해 질문 이어가겠습니다. | |||
- | |||
- 뭘 배웠는지 뭘 했는지 아직 모르겠습니다. 뭘 해야하는지도요... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
막막할때는 하나씩 해보시는건 어떨까요 ?
은영님처럼 처음 개발을 배우는 사람뿐아니라, 실무에 있는 사람이나, 고연차에 사람들도 막막할때가 있습니다.
그럴때는 조금은 멀리 떨어져서 생각해보거나, 하나하나 차근차근 생각해보는 게 저한테는 도움이 되더라고요 !
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게