Skip to content

[1,2,3단계 미션] 김동건 미션 제출합니다.#89

Merged
boorownie merged 28 commits intocho-log:rahwan10from
rahwan10:main
Apr 16, 2026
Merged

[1,2,3단계 미션] 김동건 미션 제출합니다.#89
boorownie merged 28 commits intocho-log:rahwan10from
rahwan10:main

Conversation

@rahwan10
Copy link
Copy Markdown

@rahwan10 rahwan10 commented Apr 6, 2026

안녕하세요. 임규영 리뷰어님! 프론트엔드 4기 김동건입니다.

그리디에서 어느덧 4주차 미션까지 하게됐지만 여전히 미숙한것같습니다. 그리디 템포에 잘 따라갈수있도록 열심히 하겠습니다!

react에 대해 처음 배우게 됐는데 기본적인 HTML, CSS, JS로 코드를 짜는것과 분명 비슷하면서도 결이 다름을 느꼈습니다. HTML을 만드는데 JS를 섞어서 쓰는 느낌이었어서 기존에 HTML에서 id로 받고 js에서 document로 불러와서 실행하는것보다 훨씬 쉽고 편하게 느껴졌습니다! 그리고 useState라는 함수가 너무 신기했는데 useState를 통해서 변수에 새로운 값을 할당하면 전체적으로 화면이 업데이트가 되는 점이 화면 업데이트를 하는것도 처음엔 좀 낯설었지만 굉장히 편하다고 생각합니다!

이번 미션에서 3단계인 Event Handler까지 구현을 했습니다 미션에 맞춰 순서대로 화면을 컴포넌트별로 나눠서 app하나에 다 import/export를 하였고 data에 각 음식점의 이름, 설명, 카테고리를 담은 배열을 app에 export하여 CategoryFilter에서 선택한거에 따라 useState를 사용하여 바로 바뀐 category값을 RestaurantList에 전달하여 화면이 바뀌도록 하였습니다.

그리고 아이템을 클릭하면 useState로 isOpen값을 true restarurantsDetailModal화면이 열리거나 열렸을때 백드롭이나 닫기버튼을 누르면 isOpen값을 false로 바꾸어 닫히도록 구현했습니다

리뷰어님과의 티키타카를 통해 REACT에 더 가까이 다가가고 싶습니다. 다음주 화요일까지 열심히 수정하고 더 공부하겠습니다 감사합니다!

Directory Structure

image

view image

image
4.mp4

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements a restaurant management application, including features for listing, filtering, and adding restaurants via modals. The feedback focuses on improving robustness and code quality, such as preventing default form submission, using optional chaining for safer property access, correcting HTML attribute usage, fixing CSS class name mismatches, and optimizing performance by moving static data outside of component definitions.

Comment thread src/components/AddRestaurantModal.jsx Outdated
Comment thread src/components/RestaurantDetailModal.jsx Outdated
Comment thread src/App.jsx Outdated
Comment thread src/components/AddRestaurantModal.jsx Outdated
Comment thread src/components/Header.jsx Outdated
function Header({setAddBtnOn}) {
return (
<header>
<h1 className="gnb_title text-title">점심 뭐 먹지</h1>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Header.css 파일에는 .gnb__title(언더바 2개)로 정의되어 있으나, JSX에서는 gnb_title(언더바 1개)로 작성되어 스타일이 적용되지 않습니다. 클래스명을 일치시켜 주세요.

Suggested change
<h1 className="gnb_title text-title">점심 뭐 먹지</h1>
<h1 className="gnb__title text-title">점심 뭐 먹지</h1>

Comment thread src/components/RestaurantList.jsx Outdated
Comment on lines +5 to +12
const categoryImage = {
한식: "../templates/category-korean.png",
중식: "../templates/category-chinese.png",
일식: "../templates/category-japanese.png",
양식: "../templates/category-western.png",
아시안: "../templates/category-asian.png",
기타: "../templates/category-etc.png",
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

categoryImage 객체는 정적인 데이터이므로 컴포넌트 외부에서 한 번만 선언하는 것이 효율적입니다. 현재는 리렌더링될 때마다 객체가 새로 생성되고 있습니다.

@rahwan10 rahwan10 changed the title [4주차 미션] 김동건 미션 제출합니다. [1,2,3단계 미션] 김동건 미션 제출합니다. Apr 6, 2026
Copy link
Copy Markdown

@gxuoo gxuoo left a comment

Choose a reason for hiding this comment

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

@rahwan10 👋
안녕하세요 동건님, 2주차 미션 이후로 다시 만나게 됐네요! React 처음 사용해보셨는데 어떠셨나요? Javascript 와는 친숙하면서도 다른 느낌을 받으셨을 것 같아요.

React의 useState Hook에 대해서 언급해주셨는데 저도 JS에서 React로 넘어왔을 때 가장 신기했던 것이 상태라는 개념이었던 것 같아요. 개인적으로 정말 편리하지만 그만큼 고려해야될 요소도 많다고 생각이 드는 개념 중에 하나라고 생각합니다!

1,2,3단계에 해당되는 요구사항들은 모두 만족시켜주셨네요 👏👏
처음이라 어려웠을 수도 있는데 고생 많으셨습니다.

이번에는 클린 코드 관점으로 몇 가지 코멘트를 남겨드릴테니 확인해보시고, 1단계 선택사항에 있는 module.css에 대해서 알아보시고 적용해보시는 건 어떠실까요?


.restaurant-filter {
padding: 8px;
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Image

위 이미지처럼 빨간색 금지 표시가 나오는 이유가 무엇이고, 어떤 걸 뜻하는 지 알려주시겠어요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

헉 스터디때 배운겁니다..! EOF로 알고있고 새로운 줄을 추가하지 않으면 나타납니다! 다 수정하도록 하겠습니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AddRestaurantModal, RestaurantDetailModal 2가지 컴포넌트에서 위 CSS 파일을 import 하고 있는데, 지금의 AddRestaurantModal.css 파일은 AddRestaurantModal.jsx 파일에서만 쓰일 것처럼 느껴져요.

하나의 CSS 파일을 2가지 컴포넌트가 import 하게 만든 이유가 있으신가요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

css파일에 class modal에 관한 스타일이 있는데 이게 AddRestaurantModal이랑 RestaurantDetailModal에 다 쓰여서 같이 쓰도록 했습니다. 하지만 특별히 하나의 css파일을 두 컴포넌트가 import하게 한 이유는 없습니다.. 한 컴포넌트당 하나의 css파일이 쓰이도록 수정하겠습니다!

Comment thread src/components/Header.jsx Outdated
function Header({setAddBtnOn}) {
return (
<header>
<h1 className="gnb_title text-title">점심 뭐 먹지</h1>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

gemini가 잘 확인했네요. CSS 파일에서 작성된 클래스명과 JSX에서 사용하고 있는 클래스명이 달라서 스타일이 적용되고 있지 않아요.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

넵 수정하겠습니다!

Comment thread src/components/Header.jsx Outdated
import "./styles/Header.css"
function Header({setAddBtnOn}) {
return (
<header>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Header.css를 보면 사용하지 않는 클래스가 하나 있지 않나요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

헉!! 아 그래서 뭔가 이상한점이 있었네요....ㅠ 수정하도록 하겠습니다 이상한점을 계속 느꼈는데 여기일줄은 몰랐습니다..ㅠ

Comment thread src/components/styles/Header.css Outdated

border: none;
border-radius: 8px;
/* background: transparent; */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 부분은 왜 주석처리 하셨나요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

버튼이 안보이는게 불편했어서 잠깐 보일수있게 주석처리를 했습니다..! push할때 원래대로 되돌려놓겠습니다

Comment thread src/App.jsx Outdated
function App() {
return <h1>Self-Paced React</h1>;
const [category, setCategory] = useState("전체");
const [isOpen, setOpen]=useState(false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

isOpen 이라는 변수명은 무엇을 여는 것 인지에 대해서 설명하는 부분이 부족해보여요. 조금 더 자세하게 바꿔볼까요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네! 만든 제가 다시봐도 무슨 변수인지 잘 모르겠네요.. 자세하게 변수명 바꿔보겠습니다

Comment thread src/App.jsx Outdated
}

export default App;
export default App;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기는 왜 띄어쓰기가 추가 됐나요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

잘못클릭한것같습니다..

Comment thread src/App.jsx Outdated
return <h1>Self-Paced React</h1>;
const [category, setCategory] = useState("전체");
const [isOpen, setOpen]=useState(false);
const [restaurantDetail,setDetail]=useState(" ");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 값은 ID를 받아오는 값으로 확인되는데, 이름을 보면 레스토랑의 정보를 모두 받아올 것 같아요. 다른 네이밍으로 수정해보시겠어요? gemini가 해준 리뷰도 고려해보세요~

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네 좀 더 자세하게 네이밍해보겠습니다!

Comment thread src/App.jsx Outdated
const [category, setCategory] = useState("전체");
const [isOpen, setOpen]=useState(false);
const [restaurantDetail,setDetail]=useState(" ");
const [addRestaurantOn,setAddBtnOn]=useState(false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

state 값 네이밍과 setState 값 네이밍이 아예 다른 느낌이 들어요..
React에서는 대부분 아래와 같은 관례로 네이밍을 지어요.

import { useState } from "react";
const [name, setName] = useState(null);

지금도 레스토랑 추가 모달을 여는 네이밍이라면 [addRestaurantModal, setAddRestaurantModal] 로 네이밍을 해야 이 변수가 어떤 값을 나타내는지 한 눈에 확인할 수 있겠죠?

나머지 변수, 함수도 네이밍에 대해서 다시 생각해보시면 좋을 것 같아요.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네! 알겠습니다!

Comment thread src/components/RestaurantList.jsx Outdated
<section className="restaurant-list-container">
<ul className="restaurant-list">
{restaurants.map((r) => (
<li key={r.id} onClick={()=>{setOpen(true),setDetail(r.id)} } className="restaurant">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

쉼표 연산자는 MDN 기준으로 “여러 표현식을 평가하고 마지막 값만 반환하는 표현식 문법”인데,
일반적인 로직에서는 잘 사용되지 않고 주로 for문 같은 특수한 경우에 사용돼요.

React에서는 이벤트 핸들러를 명확한 statement 형태로 작성하는 것이 가독성과 유지보수 측면에서 더 적합하기 때문에,
쉼표 연산자 대신 세미콜론(;) 또는 handler 분리를 사용하는 것이 일반적이에요! 아래 문서도 확인해보세요

쉼표 연산자 MDN

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

아 그렇군요!! 잘쓰지 않아서 몰랐는데 저런 문법이 있었네요 세미콜론으로 분리하도록하겠습니다 감사합니다!

Copy link
Copy Markdown

@gxuoo gxuoo left a comment

Choose a reason for hiding this comment

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

my-app 폴더가 생긴 이유는 뭔가요...?? 아무것도 사용하지 않는 것 같은데 존재하는 이유를 모르겠네요. 사용하지 않는 파일이면 커밋에 올리지 않아야 하고, 실수로 올렸다면 되돌리는 방법을 찾아보거나 저한테 물어봐도 좋아요. 리뷰어에게 미션을 제출할 때나 리뷰를 받고 수정한 뒤에 다시 멘션을 줄 때는 다시 한 번 체크해봅시다.

제가 깜빡하고 의존성 설치를 안하고 처음 리뷰를 드렸네요... 바보같은 실수를.. 죄송합니다😭
의존성을 설치하고 npm run lint 명령어를 실행시켜보니 총 78개의 error를 뱉고 있더군요. 그런데 그 중에 my-app이 51개의 error가 나오고 있어요. 사용하지 않으면 삭제해주세요.

개인적으로 저는 이 미션을 수행하면서 상태(State) 라는 개념을 적재적소에 잘 사용해야겠다는 느낌을 많이 받았었어요. 지금 동건님의 App.jsx 파일을 보니 이 생각이 특히 많이 났었구요. 그리고 처음이라 모든 개념이 낯설고 어렵겠지만 하다보면 익숙해질테니 너무 어려운 내용은 LLM AI 모델을 사용해서 질문하면서 학습해도 좋다고 생각합니다.

코멘트가 대부분 수정에 대한 이야기인데, 제가 언급을 안 했을 뿐, 잘해주신 내용도 충분히 존재합니다. 다만 제가 한 가지 부탁을 드리자면, 잘 해결이 되지 않는 부분이 있으면 리뷰어에게 질문을 해도 되고 아니면 충분히 고민 및 학습을 진행하신 후에 리뷰어에게 멘션을 줘도 늦지 않습니다.

항상 미션 제출 및 멘션을 주실 때는 고민했던 점이나 질문하고 싶은 내용, 어려웠던 내용들을 공유해주세요. 그러면 리뷰어가 조금 더 자세하게 봐줄 수 있는 점이 생길 것 같아요.


추가 꿀 Tip

Image

위 이미지처럼 Files changed에 들어가서 왼쪽을 보면 댓글(코멘트) 표시가 있어요. 이를 클릭해서 확인하고 답변을 달면 Start review라는 버튼이 있고 얘를 누르면 리뷰가 시작이 돼요. 그렇게 코멘트마다 답변을 다 달고 우측 상단에 Submit review를 누르고 최종적으로 할 말을 누르고 맨 아레 Submit review 버튼을 누르면 리뷰이에게 이메일로 알림이 1번 온답니다~~ 한 번 Try 해보세요 ㅎㅎ

Comment thread src/components/styles/RestaurantDetailModal.css Outdated
Comment thread src/App.jsx Outdated
Comment on lines +17 to +28
const [openDetailModal, setOpenDetailModal] = useState(false);
const handleOpenDetailModal = () => {
setOpenDetailModal(true);
};
const handleCloseDetailModal = () => {
setOpenDetailModal(false);
};

const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);
const handleFilteredREstaurantDetail = (id) => {
setFilteredRestaurantDetail(id);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

const [openDetailModal, setOpenDetailModal] = useState(false);
const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);

2가지 상태를 따로 관리하는 이유가 뭔가요? 제가 느끼기에는 RestaurantDetailModal 이라는 큰 구조 안에서 같이 관리가 되어야 하는 상태로 보여요. 동건님의 의도가 무엇인지 궁금하네요? 오타도 있구요..

위 부분을 해결하면 RestaurantList 에서 onClick 이벤트에 대해서도 간단하게 수정할 수 있을 것 같아요.

<li
  key={r.id}
  onClick={() => {
    handleOpenDetailModal();
    handleFilteredREstaurantDetail(r.id);
  }}
  className="restaurant"
>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

아 혹시 핸드러로 감싸는게 이렇게 2개의 state를 한번에 처리할때만 핸들러에 감싸서 내려주는건가요?? 제가 완전 잘못짚고있었던것같네요... 수정하겠습니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

제가 이전 리뷰에서 좀 간단하게만 짚고 넘어간 거 같아서 예시를 보여드릴게요~!

이벤트 핸들러로 감싸는 패턴이 유효한 경우

  1. 인자 변환이 필요할 때 (e.g. 이벤트 객체에서 값 추출)
const handleChange = (e) => {
  setCategory(e.target.value); // e → value 변환
};
  1. 부가 로직이 있을 때
const handleSetCategory = (filter) => {
  setCategory(filter);
  ogAnalytics(filter); // 추가 동작
};
  1. 자식에게 내려줄 때 인터페이스를 통제할 때
const handleSetCategory = (filter) => {
  setCategory(filter.id); // 자식이 넘긴 객체에서 필요한 값만 추출
};

지금 작성해주신 동건님의 코드는 인자 변환도 없고, 추가 로직도 없기에 setCategory의 역할을 동일하게 수행하고 있죠?? 이럴 때는 핸들러로 감싸줄 필요가 없어요!

핸들러로 감싸는 이유는 setCategory라는 내부 구현을 직접 노출하지 않기 위함도 있지만, 핵심은 변환이나 부가 로직이 있을 때 의미가 생겨요. 지금처럼 그대로 전달만 한다면 오히려 불필요한 간접 계층이 될 수 있으니 너무 일반화해서 생각 안 하셔도 돼요!

Comment thread .prettierrc
Comment thread .eslintrc.cjs
Comment thread src/App.jsx Outdated
Comment thread src/components/Main/RestaurantList.jsx
Comment thread src/App.jsx Outdated
Comment thread src/App.jsx Outdated
Copy link
Copy Markdown
Author

@rahwan10 rahwan10 left a comment

Choose a reason for hiding this comment

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

제가 너무 무례하게 코드 리뷰를 부탁드린것같아서 너무 죄송한 마음을 가지고있습니다.. 많이 부족하지만 앞으로 더 열심히 배워나가겠습니다.

지난번 리뷰때 state함수들을 핸들러로 감싸는게 관례라고 말씀해주셨는데 혹시 state가 2개 동시에 실행되는 경우에 핸들러로 묶어서 보내라는 뜻인가요? 제가 잘못이해해서 모든 state에 핸들러를 씌웠습니다..

그리고 푸시하는 과정에서 뭔가 꼬여서 고치니 모든 코드가 초록색이 뜨네요.. 너무 죄송합니다.. ㅠ

부족한 코드임에도 리뷰해주셔서 너무 감사합니다

Comment thread src/App.jsx Outdated
Comment thread src/App.jsx Outdated
Comment thread src/App.jsx Outdated
Comment on lines +17 to +28
const [openDetailModal, setOpenDetailModal] = useState(false);
const handleOpenDetailModal = () => {
setOpenDetailModal(true);
};
const handleCloseDetailModal = () => {
setOpenDetailModal(false);
};

const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);
const handleFilteredREstaurantDetail = (id) => {
setFilteredRestaurantDetail(id);
};
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

아 혹시 핸드러로 감싸는게 이렇게 2개의 state를 한번에 처리할때만 핸들러에 감싸서 내려주는건가요?? 제가 완전 잘못짚고있었던것같네요... 수정하겠습니다!

Comment thread .eslintrc.cjs
Comment thread src/App.jsx Outdated
Comment thread src/components/Main/RestaurantList.jsx
Comment thread .prettierrc
Comment thread src/components/styles/RestaurantDetailModal.css Outdated
@rahwan10 rahwan10 requested a review from gxuoo April 13, 2026 13:16
Copy link
Copy Markdown

@gxuoo gxuoo left a comment

Choose a reason for hiding this comment

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

무례하다뇨~~ 아니에요 ㅋㅋㅋㅋㅋ 열심히 하시는 모습이 멋있습니다 👍👍 시험기간에 처음 배우는 내용을 이렇게 하시는 것도 대단하다고 생각해요! state 핸들러에 관한 이야기는 아래에 링크에 리뷰 달아놨으니 확인해보세요!! 모든 상황에 대해서 핸들러를 적용시키는 것은 아닙니다 ㅎㅎ

깃허브는 하면서 조금씩 더 익숙해지실 거에요. 저는 예전에는 CLI 환경을 쓰다가 github desktop이라는 GUI 환경 소프트웨어를 사용하는데 이게 익숙해져서 전 이제 이것만 쓰고 있어요. 물론 개인 취향이라,, 동건님이 편하신대로 사용하시면 될 것 같아요!

리뷰 티키타카를 범수님께 수요일까지 연장해달라고 요청드렸으니, 천천히 확인하시고 궁금한 내용있으면 편하게 남겨주시고 멘션 주세요!! 끝까지 화이팅!!
(우선 Approve 처리할게요!)

핸들러 리뷰

Comment thread src/App.jsx Outdated
Comment on lines +10 to +26
const [category, setCategory] = useState('전체');

const filteredRestaurants =
category === '전체' ? restaurants : restaurants.filter((r) => r.category === category);

const [detailModal, setDetailModal] = useState(false);

const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);

const handleRestaurantDetailId = (r) => {
setDetailModal(true);
setFilteredRestaurantDetail(r.id);
};

const selectedRestaurant = restaurants.find((r) => r.id === filteredRestaurantDetail);

const [restaurantModal, setRestaurantModal] = useState(false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
const [category, setCategory] = useState('전체');
const filteredRestaurants =
category === '전체' ? restaurants : restaurants.filter((r) => r.category === category);
const [detailModal, setDetailModal] = useState(false);
const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);
const handleRestaurantDetailId = (r) => {
setDetailModal(true);
setFilteredRestaurantDetail(r.id);
};
const selectedRestaurant = restaurants.find((r) => r.id === filteredRestaurantDetail);
const [restaurantModal, setRestaurantModal] = useState(false);
// 상태값
const [category, setCategory] = useState('전체');
const [detailModal, setDetailModal] = useState(false);
const [filteredRestaurantDetail, setFilteredRestaurantDetail] = useState(null);
const [restaurantModal, setRestaurantModal] = useState(false);
// 파생값
const filteredRestaurants =
category === '전체' ? restaurants : restaurants.filter((r) => r.category === category);
const selectedRestaurant = restaurants.find((r) => r.id === filteredRestaurantDetail);
// 핸들러
const handleRestaurantDetailId = (r) => {
setDetailModal(true);
setFilteredRestaurantDetail(r.id);
};

상태 값들을 선언하고, 이에 해당되는 파생 값들을 속성에 맞게 분리해주시면 코드 가독성이 더 좋아질 것 같아요! 그리고 네이밍을 조금 더 신경써보면 어떨까 싶어요.

detailModal은 레스토랑 상세정보 모달을 관리하는 값이고, 코드를 보니까 restaurantModal이라는 값이 다음 미션인 레스토랑 추가 모달을 위한 상태 값이더라구요. 그러면 addModal 이 조금 더 직관적으로 읽히지 않을까요? 제가 코드를 처음 봤을 때는 두 상태 값이 같은 역할을 하는 느낌을 받았었어요 ㅜㅜ

스터디 때 아마 언급을 했을텐데 개발자들이 코드를 짜면서 제일 오래하는 생각이 네이밍이라고 생각해요. 예를 들어 핸들러 네이밍은 내부에서 무슨 일이 벌어지는지가 아니라, 어떤 상황에서 호출되는지를 나타내면 어떨까요?

handleRestaurantDetailId는 "ID를 다룬다"는 구현 세부사항이 이름에 노출돼 있는데, 나중에 ID 대신 객체 전체를 저장하는 방식으로 바꾸면 이름도 같이 바꿔야 되겠죠? handleRestaurantClick이나 handleSelectRestaurant처럼 사용자가 음식점을 클릭/선택했다는 행동을 이름에 담으면, 내부 구현이 바뀌어도 이름은 그대로 유효할 수 있어요.

네이밍은 언제까지나 고민하게 되는 요소라 정답...은 없는 거 같아요. 저도 네이밍 고민을 제일 오래한답니다 ㅜㅜ

Copy link
Copy Markdown
Author

@rahwan10 rahwan10 Apr 15, 2026

Choose a reason for hiding this comment

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

네이밍을 잘하도록 노력해야될것같다고 저도 느낍니다..ㅠ 저도 제 코드를 오랜만에 보면 헷갈리니까요.. 다른분들이 어떻게 네이밍 하는지 보고 , 혼자서 어떻게 해야 할지 생각하면서 계속해서 네이밍실력을 갈고닦겠습니다!

위코드의 경우 저는 filteredRestaurants가 category의 상태에서 파생돼서 category의 상태값 밑에 filteredRestaurants를 두었는데 리뷰어님 말씀대로 상태값, 파생값, 핸들러로 나누니까 훨씬 깔끔하고 보기좋은것같습니다! 감사합니다!

<section className="restaurant-list-container">
<ul className="restaurant-list">
{filteredRestaurants.map((r) => (
<div
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ui 태그 안에 div 태그를 넣은 이유가 있으실까요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

원래 li태그를 넣었는데 li태그에 onClick을 넣었을 때 non-interactive 엘리먼트에는 마우스, 키보드 이벤트리스너를 못 넣는다고 오류가 나서 div로 바꿨습니다!

Comment thread src/components/Header/Header.jsx Outdated
className="gnb__button"
aria-label="음식점추가"
>
<img src="../../../templates/add-button.png" alt="음식점 추가" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기도 import 방식으로 이미지를 불러와야겠죠?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네 그렇네요! 수정하겠습니다!

Comment thread src/components/Main/CategoryFilter.jsx Outdated
Comment on lines +16 to +22
<option value="전체">전체</option>
<option value="한식">한식</option>
<option value="중식">중식</option>
<option value="일식">일식</option>
<option value="양식">양식</option>
<option value="아시안">아시안</option>
<option value="기타">기타</option>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AddRestuarantModal 에도 같은 코드가 존재하고 있는데, 이 때 서비스 요구사항에 카테고리가 하나 추가되거나, 삭제하는 일이 발생하면 어떻게 될까요?? 여기있는 카테고리를 언제나 option 태그로 하나씩 작성하기엔 무리가 있겠죠?? 어떻게 수정하면 좋을까요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

배열 하나를 만들어서 각 파일에 import 해서 map함수로 배열에 적힌 밸류의 옵션을 만들었습니다! 이렇게 중복되는 html도 이렇게 짧게 만들 수 있으니 좋은거같아요!

Comment thread src/components/Main/RestaurantList.jsx Outdated
RestaurantList.propTypes = {
filteredRestaurants: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

 {
    id: 'a01',
    name: '피양콩할마니',
    description:
      '평양 출신의 할머니가 수십 년간 운영해온 비지 전문점 피양콩 할마니. 두부를 빼지 않은 되비지를 맛볼 수 있는 곳으로, ‘피양’은 평안도 사투리로 ‘평양’을 의미한다. 딸과 함께 운영하는 이곳에선 맷돌로 직접 간 콩만을 사용하며, 일체의 조미료를 넣지 않은 건강식을 선보인다. 콩비지와 피양 만두가 이곳의 대표 메뉴지만, 할머니가 옛날 방식을 고수하며 만들어내는 비지전골 또한 이 집의 역사를 느낄 수 있는 특별한 메뉴다. 반찬은 손님들이 먹고 싶은 만큼 덜어 먹을 수 있게 준비돼 있다.',
    category: '한식',
  },

id를 보면 문자열로 관리되고 있어요. 이에 맞춰서 string 값으로 변경해야겠죠?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

왜 저걸 number로 해놨을까요..ㅠ 수정하겠습니다!

Comment thread src/components/styles/RestaurantDetailModal.css Outdated
Copy link
Copy Markdown
Author

@rahwan10 rahwan10 left a comment

Choose a reason for hiding this comment

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

안녕하세요 임규영 리뷰어님 자세하고 정성스러운 리뷰해주셔서 너무 감사합니다 덕분에 이번에도 많이 배우는 티키타카 기간이 된것같습니다. 너무 감사드립니다! 앞으로도 열심히 적극적으로 배우겠습니다!

@rahwan10 rahwan10 requested a review from gxuoo April 15, 2026 14:22
Copy link
Copy Markdown

@gxuoo gxuoo left a comment

Choose a reason for hiding this comment

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

네 확인했습니다! 이번 미션 리뷰는 여기까지 진행하도록 할게요!
시험 공부, 다음 미션도 화이팅하세요~!

@boorownie boorownie merged commit 3598ba1 into cho-log:rahwan10 Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants