-
Notifications
You must be signed in to change notification settings - Fork 31
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
[심유빈] sprint4 #108
The head ref may contain hidden characters: "basic-\uC2EC\uC720\uBE48-sprint4"
[심유빈] sprint4 #108
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.
전반적으로 요구사항을 만족하는 과제였습니다.
자그마한 부분만 고치면 더욱 좋은 코드가 될 것 같습니다.
과제 수행하느라 고생 많으셨습니다!
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.
commit을 하실 때 node_modules
폴더는 제외하고 commit을 해 주셔야 합니다. 해당 폴더가 포함될 경우 용량이 커지고, 파일 변경사항이 많아 정확한 PR 내역을 알기 힘들어 집니다. gitignore에 대해 찾아보시면 좋을 것 같습니다
Git ignore
Ignore.io
@@ -0,0 +1,53 @@ | |||
export function getArticleList(page = 1, pageSize = 10, keyword = "") { | |||
const url = new URL("https://sprint-mission-api.vercel.app/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.
공통 URL이 반복되고 있습니다. 반복되는 상수의 경우 별도 상수로 분리하여 필요한 상황에 호출하는 식으로 사용하시면 재사용성과 코드 가독성, 확장성에서 이득이니 한 번 해 보시면 어떨까요?
const BASE_URL = "https://sprint-mission-api.vercel.app/articles";
url.searchParams.append("page", page); | ||
url.searchParams.append("pageSize", pageSize); |
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.
쿼리스트링 문자열이 아닌 배열로 관리하신 점 좋습니다!
export async function createArticle(articleData) { | ||
const res = await fetch("https://sprint-mission-api.vercel.app/articles", { | ||
method: "POST", | ||
body: JSON.stringify(articleData), | ||
headers: { | ||
"Content-Type": "application/json", | ||
}, | ||
}); | ||
if (!res.ok) { | ||
const message = await res.text(); | ||
throw new Error(message); | ||
} | ||
const data = await res.json(); | ||
return data; | ||
} |
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.
async/await 형식으로 잘 작성해 주시긴 했는데 then()
을 이용하지 않은 이유가 있으실까요?
.catch((e) => console.log(e.message)); | ||
} | ||
|
||
export async function createArticle(articleData) { |
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.
입력받는 값에 대한 검증이 있으면 더욱 좋을 것 같습니다!
"Content-Type": "application/json", | ||
}, | ||
}); | ||
if (!res.ok) { |
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
로 조건문을 걸어주셨는데 물론 알고 계시겠지만 status code에 대해서도 같이 공부해 보시면 좋을 것 같습니다!
method: "DELETE", | ||
}) | ||
.then((res) => res.json()) | ||
.catch((e) => console.log(e.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.
오류처리는 과제에는 별다른 요구사항이 없긴 하지만 console.log()
로 끝나는 것 보단 최소한 오류 코드라도 반환하는 것이 좋습니다!
basic-심유빈-sprint4
스프린트미션 4 작업입니다.
주요 변경사항
요구사항
기본요구사항
공통