-
Notifications
You must be signed in to change notification settings - Fork 0
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
[3주차 기본/심화/공유 과제] 카드 뒤집기 게임 #4
base: main
Are you sure you want to change the base?
Conversation
import Header from "../Header/Header"; | ||
|
||
export default function CardGame() { | ||
const levels = ["easy", "normal", "hard"]; |
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.
여기 levels가 props로 전달되고 있네요! 그치만 header에서만 필요한 변수인 것 같은데,
상수로 관리해도 좋고, header에서만 사용되니 header 내에서 선언해줘도 될 것 같다는 의견입니다!!
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.
저는 팟짱님이 구현예시영상으로 보여주신 부분에서 난이도 선택 버튼이 header에 있고, 현재 스코어를 보여주는 부분은 카드게임과 같은 파트에 있는게 UX상 맞다고 생각해서 두개 위치를 바꿨습니다. 그래서 현재 CardGame컴포넌트에서 Header컴포넌트와 Game컴포넌트를 자식으로 호출하고 있고, Header컴포넌트에서 버튼 클릭으로 인해 level값이 바뀌면 CardGame컴포넌트로 값이 전달된 후 Game컴포넌트로 전달되는 구조입니다. 그래서 전역으로 상태관리를 해도 좋았겠다 생각했지만 이 과제에서는 어차피 level이 바뀌면 전체적으로 렌더링이 일어나야하는 로직이라 생각해서 지금처럼 구현했던것 같아요!
근데 levels를 상수로 관리하는게 좋을것같긴 하다고 생각합니다. 로직은 그대로지만 levels부분을 상수로 빼보겠습니다!
|
||
export default function CardGame() { | ||
const levels = ["easy", "normal", "hard"]; | ||
const [selectedLevel, setSelectedLevel] = useState("easy"); |
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.
여기 "easy"가 여러군데서 쓰이고 있네요!
오타 방지 및 확장성을 위해서 상수로 분리하는 습관을 들이는 것도 좋을 것 같아요 ㅎㅎ
constants 폴더에 예시로 number라는 파일 만들어서 관리해도 좋구요!!
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.
저도 "easy"를 상수로 분리할 생각은 못 했는데 저도 배워갑니다 !!
const calculateTotalPairs = (level) => { | ||
switch (level) { | ||
case "easy": | ||
return 5; | ||
case "normal": | ||
return 7; | ||
case "hard": | ||
return 9; | ||
default: | ||
return 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.
난이도를 상수로 관리하면 여기도 똑같이 작성할 수 있겠죠!
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.
정안님 말씀처럼, 변하지 않는 상수 데이터들은 가독성과 유지보수의 용이함을 위해 따로 상수 폴더에서 관리하는게 더욱 좋을 것 같아요.
또한 컴포넌트 내 상태와 전혀 관련없는 로직이라면, 마찬가지로 util
로 !!
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.
넵 반영하겠습니다 감사합니다!
import { | ||
Title, | ||
LevelSelector, | ||
LevelButton, | ||
ResetButton, | ||
HeaderWrapper, | ||
} from "./Header.styled"; |
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.
여기는 방법의 차이이긴한데, 저 같은 경우에는
import * as S from './Header.styled'
이렇게 불러와서 앞에 S만 붙여주는 식으로 사용하고 있어요!
더 편한 방식대로 사용하심 될 것 같습니다.
다만, 제 PR에 이렇게 스타일 코드를 분리하는 것에 대한 의견을 남겨놓았으니 같이 의견 나눠보아요 ㅎㅎ
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.
S를 붙이는 이유가 styled-components로 만든 스타일 컴포넌트와 Header, Modal같은 로직을 담당하는 컴포넌트를 쉽게 구분하기 위해서 붙여주는건가요?
저도 UI를 담당하는 컴포넌트와 로직을 담당하는 컴포넌트 모두 대문자로 시작하는 컴포넌트 이름때문에 헷갈리는 경우가 많았는데 좋은 방법인것 같습니다!
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.
저도 정안님과 같은 방법 사용하고 있는데 각각의 방법에 장단점이 분명한 것 같습니다!
하나는 �간결성 측면, 하나는 필요한 것만 import 해온다는 측면에서 너무 많은 것이 필요할 때는 정안님이 추천해주신 방법이 좋을 것 같다는 생각이 듭니다.
많은 것의 기준은 잘 ...!!! 같이 의견 나눠보아요 !!
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.
어떤 방법이 더 좋을까에 대해서는 사실 고민이 많긴합니다...
화랑님 처럼 하나하나 import를 해오는 방식,
저나 진님처럼 S dot naming으로 import 해오는 방식,
다른 방법으로는 스타일 코드를 그냥 컴포넌트 내에서 작성하는 방식...
각각의 장단점이 명확한 것 같아서, 각 팀의 컨벤션에따라 충분한 논의 후 정해도 될 것 같습니다.
제가 실제로 하고 있는 고민이기도 하고, 이번 과제 PR에 고민내용을 적어놨으니 한 번쯤 살펴보셔도 좋을 것 같아요!!
<LevelSelector> | ||
{levels.map((level, i) => ( | ||
<LevelButton | ||
key={i} |
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.
저도 태승님한테 배웠는데, map을 사용할 때 key값에 index를 사용하지 않아야한다고 알고 있잖아요?
그렇지만 태승님의 레퍼런스를 보면 배열이 바뀌지 않을 때에는 key값에 index를 사용해도 된다네요!
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.
저도 딱히 key값으로 사용할게 없어서 그냥 i로 해놓고 수정해볼까 하다가 안했어요..ㅋㅋㅋ
레퍼런스 감사합니다 잘 읽어볼게요!
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.
key값을 index로 사용할 때는 static이나,static-like 배열을 사용해야하겠네요 !! 공유감사합니다.
그래서 리액트에서 static 으로 선언하는 방법이 궁금해서 찾아봤는데,
정리한 내용은 아래와 같습니다!
- 클래스 컴포넌트에서 static 배열 선언하기
import React, { Component } from 'react';
class MyComponent extends Component {
static myStaticArray = ['Apple', 'Banana', 'Cherry'];
render() {
return (
<div>
{MyComponent.myStaticArray.map((item, index) => (
<p key={index}>{item}</p>
))}
</div>
);
}
}
export default MyComponent;
위의 예제에서 myStaticArray는 MyComponent 클래스에 속한 static 배열인데,
이 배열은 클래스의 모든 인스턴스에서 동일하게 접근 가능하며, 변경되면 모든 인스턴스에 영향을 줍니다.
-
함수 컴포넌트에서 "static-like" 데이터 사용하기
함수 컴포넌트에서는 클래스와 같은 static 키워드를 직접적으로 사용할 수 없지만 유사한 기능을 구현할 수 있습니다.2-1. 모듈 스코프 변수: 함수 컴포넌트 외부에 변수를 선언하면, 이 변수는 모듈 내에서 공유되며 static 변수처럼 동작합니다.
import React from 'react';
const staticArray = ['Apple', 'Banana', 'Cherry'];
function MyFunctionalComponent() {
return (
<div>
{staticArray.map((item, index) => (
<p key={index}>{item}</p>
))}
</div>
);
}
export default MyFunctionalComponent;
2-2. useRef 사용: useRef 훅을 사용하여 함수 컴포넌트 내에서 변경 가능하지만 렌더 사이클을 트리거하지 않는 변수를 관리할 수 있습니다. 이 방법은 주로 값을 유지할 필요가 있지만 렌더링에 영향을 주고 싶지 않을 때 사용됩니다.
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 generateCards = useCallback(() => { | ||
const selectedImages = CARD_LIST.sort((a, b) => 0.5 - Math.random()).slice( | ||
0, | ||
totalPairs | ||
); |
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.5가 무엇을 의미하는지 알고 있지만,
추후에 합세나 웹잼때에는 0.5가 무엇을 의미하는지 constant에서 의미있는 네이밍을 줘도 좋을 것 같아요!
제 코드에서 constant를 한 번 참고해보세요 ㅎㅎ!
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.
여기에 useCallback을 사용한 이유가 궁금합니다!!
저는 useCallback에 미숙해서 배우고 싶어요 ㅎㅎ!
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.
특정 함수가 props
를 통해 다른 컴포넌트로 전달되는 것이 아니라, 해당 컴포넌트에서만 존재한다면 useCallback
은 큰 의미가 있지는 않을 것 같아요.
차라리 값비싼 연산을 최적화하기 위해 사용하셨다면 useMemo
가 더욱 적합할 것 같다고 개인적으로 생각합니다 !
보통 useCallback
은 해당 훅으로 감싼 함수가 다른 컴포넌트로 prop
으로 전달될 때 최적화를 위해 사용하는 것으로 알고 있습니다 ! 왜냐면 해당 훅으로 감싸지지 않는다면, 해당 컴포넌트의 다른 상태가 변경될때마다 계속하여 함수가 재생성되기 때문입니다.
재생성된 함수는 이전 함수와는 다른 객체이기 때문에, 해당 함수를 prop으로 전달받는 컴포넌트에서 또한 리액트는 다른 prop으로 인식하는 것으로 알고 있어요 ! 따라서 해당 함수를 캐시 처리하여 최적화하기 위해 useCallback
을 사용하는 것이고요.
물론 이 과정이 의미가 있기 위해서는 자식 컴포넌트 또한 memo
처리를 해주어, 같은 prop
이라면 리렌더링 하지 않도록 처리해두어야 의미가 있을 것 같습니다.
참고링크 남깁니다 !
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.5를 다른 팀원이 보기에는 어떤 의미인지 파악하기 어려울것 같아요.
혹시 constant로 빼는 방식말고 간단하게 밑에 주석으로 설명을 적는건 어떨지 궁금합니다!
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.
제가 useCallback을 제 코드에 직접 도입해보는건 처음이어서 적절한 활용이 아니었던것 같아요. 주용님 말씀대로 뭔가 onClick처럼 props로 다른 컴포넌트에 전달해주는 로직이 아니라 단순히 카드배열을 섞고 그중 totalPairs개수만큼 추출해내는 로직이라 useMemo를 사용하는게 더 좋을것 같네용 감사합니다!
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.
반영했습니당~
return shuffledcards; | ||
}, [totalPairs]); | ||
|
||
const handleCardClick = (id) => { |
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.
여기 부분 if문 충분히 줄일 수 있을 것 같아요!
예를 들어,
if(selected.isOpen || selected.isMatched) return;
...관련 로직
if (selectedCard === null) {
setSelectedCard(selected);
} else {
if (selectedCard.imgSrc === selected.imgSrc) {
setTimeout(() => {
setCards((prevCards) =>
prevCards.map((card) =>
card.imgSrc === selected.imgSrc ? { ...card, isMatched: true } : card
)
);
setSelectedCard(null);
setMatchedScore((prev) => prev + 1);
}, 500);
} else {
setTimeout(() => {
setCards((prevCards) =>
prevCards.map((card) =>
card.isOpen ? { ...card, isOpen: false } : card
)
);
setSelectedCard(null);
}, 500);
이런 느낌이면 if문 뎁스를 하나 이상 줄일 수 있겠죠??
가독성을 위해 한 번 고민해보시길 ㅎㅎ
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.
자세한 코드 설명 감사합니다! 비슷한 느낌으로 반영했습니다!
resetClicked={resetClicked} | ||
afterCardReset={afterCardReset} |
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.
여기서 afterCardReset은 props로 내려주지 않고, setResetClicked만 내려준 다음,
Game 컴포넌트에서 afterCardReset을 만들어줘도 좋다는 입장입니다!
props로 많이 내려주는 느낌이라서 ㅎㅎ...
다른 분들은 어떻게 생각하시는지 궁금합니다 ㅎㅎ!
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.
저는 다르게 생각해요 ! setResetClicked
은 오직 상태를 변경하는 setter 함수일 뿐 어떠한 역할을 수행하는 순수 함수로서는 동작하지 못한다는 생각이 들어요.
따라서 Game 컴포넌트가 도메인을 포함하는 만큼, 특정 도메인에서 어떠한 역할을 담당하는지에 대한 함수로 작성해주어 props
로 내려주는 것이 더욱 좋다고 생각합니다.
저는 보통 useState
의 상태 변경 함수를 그대로 props
로 내리는 행위는 잘 하지 않는데 다른 분들의 의견 듣고 싶네요 ㅎㅎ
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.
저도 상태 변경 함수를 그대로 props로 내리는 방법은 잘 선호하지 않아서...
그럼 주용님은 보통 상태 변경 함수를 그대로 props로 내리는 방법은 택하지 않는다고 하셨는데,
Game 컴포넌트가 도메인을 포함하니 어떤 역할을 하는지 함수 네이밍을 새로 해서 props로 내리는게 좋다는 의견이실까요?
이런 상황이라면 보통 어떤 방법을 택하시는지, 조금 더 자세하게 듣고 싶어요 ㅎㅎ!
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.
저도 setter함수를 그대로 props로 내리는게 건강한 로직이 아니라고 배워서 afterCardReset을 불필요하다면 불필요할수도 있겠지만 만들어서 콜백함수로 전달해줬습니다.
리셋버튼은 Header 컴포넌트에서 클릭되고 있고, 카드가 리셋된 후에 다시 resetClicked를 false로 만들려면 Game컴포넌트에서 카드 재생성, 매치된 카드쌍을 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.
맞습니다 ! 저는 특정 도메인을 포함하거나, 만약 도메인이 분리된 범용 컴포넌트라고 해도 setter
함수를 그대로 전달해주지는 않는 것 같아요.
하지만 제가 정답은 아닌 것 같다고 생각합니다. 왜냐면 리액트 개발자들이 setter
함수를 그대로 내려주는 행위를 따로 안티 패턴이라고 칭한 걸 본 적은 없는 것 같아요 ! 따라서 제 개인적인 생각으로 받아주시면 좋을 것 같네요 :)
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.
저는 항상 setter 함수를 그대로 넘겨주곤 했는데 이런 방식 있다는 것도 배워갑니다 !!
이 기회에 곰곰히 생각해보니,
setter 함수를 그대로 넘겨줄 경우,
자식 컴포넌트가 상태를 변경할 때, 어떤 상태가 어떻게 변경되는지 부모 컴포넌트가 세밀하게 제어하기 어려울 것 같습니다.
자식 컴포넌트에서 임의로 상태를 변경할 수 있어 예기치 않은 부작용이 발생할 수 있는데,
화랑님 방식대로 사용자 정의 함수를 만들어 전달하게 되면 부모 컴포넌트에도 어느정도 제어권이 생기는 장점이 생길 것 같아요!
비록, 코드 복잡성이나 오버헤드가 증가하는 단점 있지만요 ...!
간단한 상태 변경이 필요한 경우에는 setter 함수를 직접 전달하는 반면,
변경 로직이 복잡하거나 여러 컴포넌트에서 동일한 방식으로 상태를 변경해야할 때는 화랑님 방식대로 사용자 정의 함수를 통해 캡슐화하는 것이 더 좋다고 생각합니다.
다른 분들 의견도 궁금합니다!
onClick={() => { | ||
onClick(); | ||
}} | ||
> |
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.
onClick={onClick}
위 코드와 동일할 것 같은데 화랑님처럼 작성하신 이유가 있으실까요??
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.
()=>{}
이 로직이 그냥 습관이어서 저렇게 한거같은데 onClick={onClick} 이 더 보기 좋아보이네요 수정하겠습니다~
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.
동감하는 바입니다 !
resetClicked={resetClicked} | ||
afterCardReset={afterCardReset} |
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.
저는 다르게 생각해요 ! setResetClicked
은 오직 상태를 변경하는 setter 함수일 뿐 어떠한 역할을 수행하는 순수 함수로서는 동작하지 못한다는 생각이 들어요.
따라서 Game 컴포넌트가 도메인을 포함하는 만큼, 특정 도메인에서 어떠한 역할을 담당하는지에 대한 함수로 작성해주어 props
로 내려주는 것이 더욱 좋다고 생각합니다.
저는 보통 useState
의 상태 변경 함수를 그대로 props
로 내리는 행위는 잘 하지 않는데 다른 분들의 의견 듣고 싶네요 ㅎㅎ
// export const Header = styled.header` | ||
// background-color: ${({ theme }) => theme.colors.SkyBlue}; | ||
// width: 100dvw; | ||
// padding: 1rem; | ||
// display: flex; | ||
// flex-direction: column; | ||
// align-items: center; | ||
// gap: 2rem; | ||
// `; | ||
|
||
// export const Title = styled.h1` | ||
// color: ${({ theme }) => theme.colors.White}; | ||
// font-family: var(--font-sunflower); | ||
// font-size: 4rem; | ||
// `; | ||
|
||
// export const LevelSelector = styled.div` | ||
// display: flex; | ||
// gap: 1rem; | ||
// `; | ||
|
||
// export const LevelButton = styled.button` | ||
// background-color: ${({ value, selectedlevel, theme }) => | ||
// value === selectedlevel ? theme.colors.LightPurple : theme.colors.Violet}; | ||
// color: ${({ theme }) => theme.colors.White}; | ||
// width: 10rem; | ||
// height: 4rem; | ||
// border-radius: 5px; | ||
// font-family: var(--font-sunflower); | ||
// font-size: 2rem; | ||
// `; | ||
|
||
// export const ResetButton = styled.button` | ||
// background-color: ${({ theme }) => theme.colors.Pink}; | ||
// color: white; | ||
// position: fixed; | ||
// top: 1rem; | ||
// right: 1rem; | ||
// width: 8rem; | ||
// height: 5rem; | ||
// border-radius: 5px; | ||
// font-family: var(--font-sunflower); | ||
// font-size: 2.5rem; | ||
// `; |
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.
아 실수로 안지웠네용 지우겠습니당~
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.
Game 컴포넌트에서 많은 비즈니스 로직뿐 아니라 UI까지 담당하고 있는 것 같아요. 커스텀 훅과 같은 패턴을 통해서 비즈니스 로직을 컴포넌트에서 분리해준다면 더욱 깔끔한 컴포넌트가 될 것 같다는 생각입니다 !
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.
저도 동의합니다!
기능과 UI를 분리해보는 연습도 좋을 것 같아요!
주용님 말씀대로 분리한다면 훨씬 한 눈에 들어오는 컴포넌트가 되겠죠?
한 번 고민해보시길 ㅎㅎ
const calculateTotalPairs = (level) => { | ||
switch (level) { | ||
case "easy": | ||
return 5; | ||
case "normal": | ||
return 7; | ||
case "hard": | ||
return 9; | ||
default: | ||
return 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.
정안님 말씀처럼, 변하지 않는 상수 데이터들은 가독성과 유지보수의 용이함을 위해 따로 상수 폴더에서 관리하는게 더욱 좋을 것 같아요.
또한 컴포넌트 내 상태와 전혀 관련없는 로직이라면, 마찬가지로 util
로 !!
const generateCards = useCallback(() => { | ||
const selectedImages = CARD_LIST.sort((a, b) => 0.5 - Math.random()).slice( | ||
0, | ||
totalPairs | ||
); |
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.
특정 함수가 props
를 통해 다른 컴포넌트로 전달되는 것이 아니라, 해당 컴포넌트에서만 존재한다면 useCallback
은 큰 의미가 있지는 않을 것 같아요.
차라리 값비싼 연산을 최적화하기 위해 사용하셨다면 useMemo
가 더욱 적합할 것 같다고 개인적으로 생각합니다 !
보통 useCallback
은 해당 훅으로 감싼 함수가 다른 컴포넌트로 prop
으로 전달될 때 최적화를 위해 사용하는 것으로 알고 있습니다 ! 왜냐면 해당 훅으로 감싸지지 않는다면, 해당 컴포넌트의 다른 상태가 변경될때마다 계속하여 함수가 재생성되기 때문입니다.
재생성된 함수는 이전 함수와는 다른 객체이기 때문에, 해당 함수를 prop으로 전달받는 컴포넌트에서 또한 리액트는 다른 prop으로 인식하는 것으로 알고 있어요 ! 따라서 해당 함수를 캐시 처리하여 최적화하기 위해 useCallback
을 사용하는 것이고요.
물론 이 과정이 의미가 있기 위해서는 자식 컴포넌트 또한 memo
처리를 해주어, 같은 prop
이라면 리렌더링 하지 않도록 처리해두어야 의미가 있을 것 같습니다.
참고링크 남깁니다 !
<LevelSelector> | ||
{levels.map((level, i) => ( | ||
<LevelButton | ||
key={i} |
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.
배열에는 각 요소의 구별자로 활용하기 위해 id
같은 프로퍼티를 설정해주시는 것은 어떨까요 ?? 배열을 순회할 경우가 많은데, 이때 key
값으로 활용하기도 좋을 것 같습니다.
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.
Game컴포넌트에서 카드리스트를 섞은다음 동적으로 id, isOpen, isMatched 속성을 추가해주고 있어서 여기엔 일부러 id 속성을 주지 않았습니다.
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.
최소한의 필요한 요소에만 글로벌 스타일을 적용하신 것 굉장히 좋은 것 같습니다 !
저도 항상 reset
만 사용해왔었는데, 이는 성능 면에서 그리 좋지 않다고 하더라고요 ! 따라서 저도 최근에 정말 필요한 요소에만 글로벌 스타일을 적용하도록 노력하고있습니다.
잘 보고 갑니다 !
week3/week3-assignment/index.html
Outdated
@@ -0,0 +1,13 @@ | |||
<!doctype html> | |||
<html lang="en"> |
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.
"ko"
3주차 카드 뒤집기 게임까지 수고 많았습니다 ㅎㅎ 화면이 잠깐 깜빡이며 저희가 구현하려고 했던 요소가 보일 때가 바로 그 때인데요, 또한 커스텀 훅으로 기능을 위임하는 측면과, 유틸함수, 스타일 코드 등 분리한다면 아주 클린한 코드가 되겠는데요? |
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 selected = cards.find((card) => card.id === id); | ||
if (!selected.isOpen && !selected.isMatched) { | ||
//첫번째 카드 선택 | ||
if (selectedCard === null) { | ||
setSelectedCard(selected); | ||
const updatedCards = cards.map((card) => | ||
card.id === id ? { ...card, isOpen: true } : card | ||
); | ||
setCards(updatedCards); | ||
} | ||
//두번째 카드 선택 | ||
else { | ||
const updatedCards = cards.map((card) => | ||
card.id === id ? { ...card, isOpen: true } : card | ||
); | ||
setCards(updatedCards); | ||
if (selectedCard.imgSrc === selected.imgSrc) { | ||
setTimeout(() => { | ||
const matchedCards = updatedCards.map((card) => | ||
card.imgSrc === selected.imgSrc | ||
? { ...card, isMatched: true } | ||
: card | ||
); | ||
setCards(matchedCards); | ||
setSelectedCard(null); | ||
setMatchedScore((prev) => prev + 1); | ||
}, 500); | ||
} else { | ||
setTimeout(() => { | ||
const resetCards = updatedCards.map((card) => | ||
card.isOpen ? { ...card, isOpen: false } : card | ||
); | ||
setCards(resetCards); | ||
setSelectedCard(null); | ||
}, 500); | ||
} | ||
} |
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 updateCards = () => {
const updatedCards = cards.map((card) =>
card.id === id ? { ...card, isOpen: true } : card
);
return updatedCards;
}
const selectCards = () => {
if (selectedCard === null) {
setSelectedCard(selected);
updateCards();
setCards(updatedCards);
}
//두번째 카드 선택
else {
updateCards();
setCards(updatedCards);
}
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.
이것도 정말 좋은 방식이네요 참고해보겠습니다!
shuffledcards = shuffledcards.map((image, index) => ({ | ||
id: index, | ||
imgSrc: image.imgSrc, | ||
imgAlt: image.imgAlt, | ||
isOpen: false, | ||
isMatched: false, | ||
})); |
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.
이렇게 객체를 만들어서 초기 값을 지정 해줄 수도 있겠네요! 배워갑니다~!
colors, | ||
}; | ||
|
||
export default theme; |
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.
theme이랑 GlobalStyles 가 잘 정리되어있는것 같아요! styles 폴더에 나란히 있어서 폴더 구조 측면에서도 좋아보입니다. 잘 배워갑니다!!!
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.
코드 가독성이 정말 좋은 코드인 것 같아요!
컴포넌트도 잘 나눠져있는 것 같고 파일 각각이 길이도 적당한 것 같습니다.
특히 setter 함수를 그대로 넘기는 게 아니라, 부모컴포넌트의 제어권을 위해 사용자정의 함수를 따로 만들어 넘기는 부분이 인상적이었어요!
저는 아예 Button 컴포넌트를 따로 분리했는데
그냥 CSS 만으로 분리해도 재사용성도 만족하면서 깔끔하네요
많은 부분에서 배울 점이 많았습니다!
이번 주차 고생 많으셨습니다 :)
✨ 구현 기능 명세
🧩 기본 과제
전체적인 game flow
header 구현
card section 구현
카드가 선택되었을 때 카드를 open합니다.
두 번째 카드가 클릭될 때 까지 카드는 open상태여야합니다.
두 번째 카드를 클릭하였을 때
정답일 경우 카드 두개를 open 상태로 고정하고 score를 올립니다.
오답일 경우 카드 두개를 다시 close합니다.
게임에 성공하였을 때
구매 모달 구현
🔥 심화 과제
난이도 설정
게임 reset
카드 뒤집기 애니메이션
모달
공유과제
링크 첨부(팀 블로그 링크) : 공유과제 링크
📌 내가 새로 알게 된 부분
공유과제로 상태관리와 데이터 흐름, 그리고 불필요한 렌더링을 막는 방법에 대해 알아보면서
useMemo
,useCallback
,React.memo
이 세가지의 개념에 대해 고민해보고 공부해보았습니다. 아직 이 세가지 개념을 제 코드에 반영해본적은 없고, 간단한 개념정도만 알고 있었는데 이번 과제를 진행하면서 좀 더 자세히 알게 되었습니다. 이를 카드뒤집기 코드에 적용시켜Game
컴포넌트에서 일어나는 불필요한 렌더링을 막아보자 생각했지만 이를 막기위해 useCallback을 사용했을 때의 메모리 소모가 오히려 악영향을 끼칠 수 있다고 생각하여 useEffect 자체를 건들이지 않고 그 내부에서 호출되는 함수 중generateCards
함수를 useCallback을 사용하여 최적화 해보았습니다.추가적으로 공부를 하면서 리액트는 렌더링 과정에서 2가지 페이즈를 거친다는것을 배웠습니다.
React.memo
를 통해 컴포넌트를 렌더하는것 자체를 최적화 할 수 있음)useMemo
,useCallback
을 사용하여 변수나 함수를 최적화 할 수 있음💎 구현과정에서의 고민과정(어려웠던 부분) 공유!
Game
컴포넌트에서 의존성배열로level
,totalPairs
를 받는 useEffect문이 있습니다. 현재 level버튼 클릭으로 인해 level값이 변경되면 useEffect가 실행되면서 totalPairs값이 변하게 되고, totalPairs값이 변했기 때문에 다시한번 useEffect문이 실행되면서 총 두번 렌더링이 일어납니다.제가 생각했을 때 level값에 변화가 생기면 totalPairs값은 무조건 변경된다 생각하여 totalPairs는 의존성배열에 넣지않고 실행을 시켜서 totalPairs값을 콘솔에 찍어보면 올바른 값(내가 생각하기에 나와야 하는 값)이 나오지만 화면상에 렌더링된 부분에서는 totalPairs가 적용되지 않는 문제가 발생했습니다.
이유는 깨달았습니다. useEffect문 특성상 컴포넌트가 렌더링이 일어난 후, 화면에 paint되고, useEffect문이 실행됩니다. 따라서 화면에 paint된 결과에는
totalPairs
의 변경된 값이 적용되지 않는것입니다. 이를 해결하기 위해서는useLayoutEffect
문을 사용해서 화면에 paint되기 전 useLayoutEffect문을 실행시켜주면 적용이 될것입니다. 하지만useLayoutEffect
문은 성능을 저하시킬 가능성이 있어 사용을 권장하지 않는다고 합니다. 그래서 사용하지 않고 두번 렌더링되도록 나뒀습니다.이를 해결할 수 있는 방법이 무엇일지 좀 더 고민해보겠습니다.
🥺 소요 시간
24h
🌈 구현 결과물
구현 결과물 넣어놓은 노션 링크