-
Notifications
You must be signed in to change notification settings - Fork 24
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
React 김현묵 sprint6 #103
The head ref may contain hidden characters: "react-\uAE40\uD604\uBB35-sprint6"
React 김현묵 sprint6 #103
Conversation
"dotenv": "^16.4.5", | ||
"express": "^4.21.1", | ||
"mongoose": "^8.7.3", | ||
"nodemon": "^3.1.7" |
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.
nodemon 같은 툴은 devDependencies로 들어가는 게 더 적절해보입니다.
무슨 차이인지 모르겠다면 npm 공식문서나 블로그 설명을 찾아보세요!
import product from "./product/controller.js"; | ||
|
||
const router = express.Router(); | ||
router.use("/product", product); |
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.
api 경로를 지정할 때 resource collection이 변경된다면 복수형의 이름을 많이 사용합니다.
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.
service 파일은 비즈니스 로직을 구현해놓은 파일인 것 같은데, routes 디렉터리 밑에 두기엔 관심사가 서로 일치하지 않는 것 같네요.
@@ -0,0 +1,106 @@ | |||
import Product from "../../model/product.js"; | |||
const Items = async (req, res) => { |
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.
함수 네이밍이 대문자로 시작하는 건 어색하게 보이네요.
let pageSize = !!query.pageSize ? Number(query.pageSize) : 10; | ||
let page = !!query.page ? Number(query.page) : 1; | ||
let orderBy = {}; |
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.
값을 재할당하는 변수가 아니라면 const로 선언하는 게 좋아보입니다.
let으로 선언하면 코드를 읽는 사람들로 하여금 해당 코드 스코프 내에서 변수 값이 언제 바뀌는지 계속 긴장하게 만들 수 있습니다.
}, | ||
{ $set: req.bdoy } | ||
); | ||
res.status(203).send({ |
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.
status code 203은 어떤 의도로 지정하신 건가요?
} catch (error) { | ||
console.error(error); | ||
} |
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.
예외가 발생하는 경우를 처리하기 위해서 try catch 구문을 꼼꼼히 사용하고 있는 부분은 좋은 접근이라고 생각됩니다. 그런데 실제로 에러가 발생한다면 API 관점에서 response는 어떻게 되는지도 고려해보셨을까요?
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.
사용하지 않을 거라면 깔끔하게 지우는 게 좋아 보입니다.
function HomeSection({ h4, h2, p, img, left }) { | ||
left = left ? "left" : ""; | ||
return ( | ||
<div className="section"> | ||
<div className="content"> | ||
{!!!left ? <img src={img} alt={h4} /> : null} | ||
<div className={`txtBox ${left}`}> | ||
<div class="abs"> | ||
<h4>{h4}</h4> | ||
<h2> | ||
{h2[0]} | ||
<br /> | ||
{h2[1]} | ||
</h2> | ||
<p> | ||
{p[0]} <br /> | ||
{p[1]} | ||
</p> | ||
</div> | ||
</div> | ||
{left ? <img src={img} alt={h4} /> : null} | ||
</div> | ||
</div> | ||
); | ||
} |
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.
HomeSection이 항상 해당 값들을 props로 받을 거라고 가정하고 있기 때문에 결합도는 높고 확장성은 떨어지는 구조입니다. 이런 형태라면 컴포넌트를 여러 개로 나누고, children으로 컴포넌트를 렌더하는 방식이면 더 괜찮을 것 같습니다.
컴파운드 패턴이라는 것도 참고해보시면 좋을 것 같습니다. (필수로 적용해야 하는 것 아님)
const naviLimit = 5; | ||
const [sellLimit, setSellLimit] = useState(10); | ||
const sellProduct = useItmeList([], sellLimit); | ||
const [sellItmeSize, setSellItemSize] = useState("220px"); |
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.
사이즈를 string 상태로 관리한다는 게 마음이 조금 불편하네요. px 단위라도 사용하는 곳에서 붙이는 걸로 하고 값은 number 타입으로 보면 어떨까요?
최종적으로는 해당 라인을 CSS 쪽으로 넘기는 방향이면 좋을 것 같습니다.
요구사항
기본
공통
프론트엔드 구현 요구사항
랜딩 페이지
중고마켓 페이지
상품 등록 페이지
백엔드 구현 요구사항
중고마켓
심화 요구사항
프론트엔드 구현 요구사항
상품 등록 페이지
멘토에게