[길도현] sprint2 #3
Conversation
shinYeongHyeon
requested review from
shinYeongHyeon
and removed request for
mag123c
| class Article { | ||
| title = ''; | ||
| content = ''; | ||
| writer = ''; | ||
| likeCount = 0; | ||
| createAt = ''; | ||
|
|
||
| constructor(createAt) { | ||
| this.createAt = createAt; | ||
| } | ||
|
|
||
| like() { | ||
| this.likeCount++; | ||
| } | ||
| } |
There was a problem hiding this comment.
클래스의 캡슐화가 진행되지 않았네요!
private 화 를 진행하기 위해서 # 을 진행하는건 어떨까요,
더불어 getter 와 setter 를 통해서, 변수를 받거나, 건네주는 게 필요해 보입니다
| } | ||
| } | ||
|
|
||
| const link = 'https://panda-market-api-crud.vercel.app/articles'; |
There was a problem hiding this comment.
위 URL 에서 https://panda-market-api-crud.vercel.app 는 반복적으로 사용 되네요.
변수를 따로 생성하여 템플릿 리터럴 을 활용하면 가독성과 유지보수 편이성을 모두 챙길 수 있을듯해요
// 다른파일
export const BASE_URI = 'https://panda-market-api-crud.vercel.app';
// 현재
const link = `${BASE_URI}/articles`;
|
|
||
| let result = fetch(address) | ||
| .then((res) => { | ||
| if(!(res.status >= 200 && res.status < 300)) console.error("응답 코드 에러 : " + res.status); |
There was a problem hiding this comment.
위와 같이 HttpStatusCode 2xx 대가 아님을 필터하여, 응답을 진행하는 것은 좋아보입니다 👍🏼
여기서 반복적으로 사용되는 !(res.status >= 200 && res.status < 300) 이 조건을 함수로 따로 빼서, 함수화 하여 의미를 쉽게 표현하면 더 좋은 코드가 될 수 있을 것 같아요
| createAt = ''; | ||
|
|
||
| constructor(createAt) { | ||
| this.createAt = createAt; |
There was a problem hiding this comment.
생성자는 createdAt 만 받고 있네요 !
다른 것도 받아서 인스턴스 생성이 편해지는건 어떨가요
| export async function deleteProduct() { | ||
| let address = url; | ||
| address.searchParams.append('productId', 1); | ||
| try { |
There was a problem hiding this comment.
github 의 오류일수도 있으나, 만약 실제로 try 의 들여쓰기가 위와 같이 돼있다면, 가독성이 조금 떨어질 것 같습니다 !
이런것들을 편하게 잡기 위해서 eslint 혹은 prettier 같은 정적도구를 활용해보는건 어떨까요 ?
| const link = 'https://panda-market-api-crud.vercel.app/products'; | ||
| const url = new URL(link); | ||
|
|
||
| let products = []; |
There was a problem hiding this comment.
해당 변수는 push 만 될 뿐, 변화가 일어나지는 않는 것 같습니다 !
따라서, const 로 선언하는 것은 어떨까요 ?
그리고 사용되는 scope 안에서 선언되는 것이 조금 더 안정적인 프로그래밍 방법이 될 듯 합니다 !
| for(let item of json.list) { | ||
| if(!item.tags.includes('전자제품')) { | ||
| products.push(new Product( | ||
| item.name, | ||
| item.description, | ||
| item.price, | ||
| item.tags, | ||
| item.images, | ||
| )); | ||
| } | ||
| else { | ||
| products.push(new ElectronicProduct( | ||
| item.name, | ||
| item.description, | ||
| item.price, | ||
| item.tags, | ||
| item.images, | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
위 부분 for of 을 활용함으로써, 받은 리스트를 배열에 잘 넣어주셨습니다,
다만 여기서 map 을 활용하는 것이 실무에서는 더 좋은 코드라고 말할 수 있겠습니다 !
| console.error("에러 발생 : " + e); | ||
| } | ||
| for(let item of json.list) { | ||
| if(!item.tags.includes('전자제품')) { |
There was a problem hiding this comment.
전자제품 과 같은 매직스트링의 경우 변수로 따로 빼서, 그 의미를 설명해준다면 좋을 것 같아요!
| let data = await fetch(url.toString(), { | ||
| method: "POST", | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| images: [ | ||
| "https://example.com/..." | ||
| ], | ||
| tags: [ | ||
| "전자제품" | ||
| ], | ||
| price: 0, | ||
| description: "string", | ||
| name: "상품 이름" | ||
| }), | ||
| }); |
There was a problem hiding this comment.
전체적으로 들여쓰는 부분들이 명확하지 않아, 가독성이 많이 떨어지는 듯 합니다,
해당 부분을 개선하면 보기 쉬운 코드가 될 것 같아요 !
요구사항
클래스 구현하기
Article 요청 함수 구현하기
Product 요청 함수 구현하기
주요 변경사항
멘토에게