Skip to content
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

MISSION2 / 최규민 #16

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gyuminzzz
Copy link
Collaborator

@gyuminzzz gyuminzzz commented Mar 27, 2024

1. 구현 모습

image

-2. 쿠폰을 적립하는 '커피 결제' 버튼을 10번 누르면 '쿠폰 사용' 버튼으로 변합니다.
image

-3. '쿠폰 사용' 버튼을 누르면 처음 단계로 되돌아가 '커피 결제' 버튼이 활성화 됩니다.
image

2. 해결 과정

선언

  • 쿠폰 개수를 저장하는 couponNum

  • '쿠폰 사용' 버튼을 활성화시키는 useCoupon

  • '커피 결제' 버튼을 활성화시키는 orderCoffee

  • 쿠폰이 10개가 되어도 쿠폰을 사용하지 않고 계속 커피 결제를 하는 경우를 어떻게 처리할지 고민해보았습니다. 쿠폰 10개를 모으면 자동으로 커피 교환권으로 바꿔주고, 커피 결제에는 간섭하지 않는 방법을 생각해보았으나 아직 제 수준에서는 복잡해서..! 그대신

  • 쿠폰이 10개가 되면 커피 결제 버튼을 비활성화, 쿠폰 사용 버튼을 활성화
    const buyCoffee = () => { setCouponNum(couponNum+1); if (couponNum === 10){ setUseCoupon(true); setCouponNum(0); setOrderCoffee(false); }
    하여 강제로 쿠폰을 사용하고 한 루틴을 돌게 했습니다.


3. To 리뷰어에게

  • 쿠폰을 사용하고, 이를 위한 버튼을 활성화하기 위한 로직을,
    또 커피를 결제하고, 이 버튼을 활성화하기 위한 로직을 이렇게 분리하는 게 맞을지 (최선인지..?) , 혹은 더 좋은 방법이 있는지 조언 해주시면 감사하겠습니다.

  • 또 아쉬웠던 점은, 커피 결제의 로직에서
    {!orderCoffee ? null : <button onClick={() => {buyCoffee()}}> 커피 결제 </button>}
    저는 평소에 ~하면 ~하다 식으로 코드를 짜고싶은데 ~아니면 ~하다 식으로 코드를 짜게돼서 신경이 쓰입니다... 보통 이런식으로도 코드를 짜시나요?

  • 바쁜 시간 내어 도와주셔서 진심으로 감사드립니다! 좋은 한 주 보내세요 🙂


@geongyu09 geongyu09 assigned geongyu09 and gyuminzzz and unassigned geongyu09 Mar 28, 2024
@geongyu09 geongyu09 added the mission 미션 입니다! label Mar 28, 2024
@2yunseong 2yunseong requested a review from ummaeha April 30, 2024 13:22
Copy link
Collaborator

@2yunseong 2yunseong left a comment

Choose a reason for hiding this comment

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

안녕하세요 규민님! 회장단 업무보랴 시험보느라 고생하셨습니다!

쿠폰을 사용하고, 이를 위한 버튼을 활성화하기 위한 로직을,
또 커피를 결제하고, 이 버튼을 활성화하기 위한 로직을 이렇게 분리하는 게 맞을지 (최선인지..?) , 혹은 더 좋은 방법이 있는지 조언 해주시면 감사하겠습니다.

또 아쉬웠던 점은, 커피 결제의 로직에서
{!orderCoffee ? null : <button onClick={() => {buyCoffee()}}> 커피 결제 }
저는 평소에 ~하면 ~하다 식으로 코드를 짜고싶은데 ~아니면 ~하다 식으로 코드를 짜게돼서 신경이 쓰입니다... 보통 이런식으로도 코드를 짜시나요?

두 개를 한꺼번에 답변해드리자면, 일단 아래에도 남겼지만 불필요한 상태가 있다고 생각합니다.(아니라면 코멘트 남겨주세요) 상태는 강력한 도구지만 안 쓸 수 있는 상황에서는 안쓰는게 좋은 양날의 검(?) 같은 거라고 생각해요. 그래서 불필요한 상태를 어떻게 합칠 수 있는 지 먼저 고민해보시면 많은 공부가 될거에요!

두 번째는 아쉬운 부분은 아니고 잘 접근하셨다고 생각이 들어요 ! 조건부 렌더링 꽤나 자주 쓰는 기법입니다. 어떻게 보면 JSX의 강력한 점이라고 생각하기 때문에, 필요한 부분이라면 사용하셔야 된다고 생각합니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

서브모듈?!

Comment on lines +4 to +6
return <div>
<MyCard/>
</div>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

불필요한 div라고 생각합니다!

Comment on lines +5 to +6

function App() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

lint나 prettier가 적용되지 않았네요...!
다음 미션부터는 꼭 적용하시길


function App() {
const [couponNum, setCouponNum] = useState(0);
const [useCoupon, setUseCoupon] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

관례상 hook에만 use를 붙입니다!

Comment on lines +9 to +10
const [orderCoffee, setOrderCoffee] = useState(true);

Copy link
Collaborator

Choose a reason for hiding this comment

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

관례상 boolean값에는 is~ has~ 등의 접두사를 붙이면 이해하기 편할 것 같아요

Comment on lines +14 to +16
setUseCoupon(true);
setCouponNum(0);
setOrderCoffee(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금 코드를 보니 useCoupon과 orderCoffee의 상태가 함께 변경되고 있네요!
그렇담 상태가 불필요하게 사용되고 있는 상황 같은데 ...! 한번 고민해보시지요

<div>{couponNum}</div>

<div>
{!orderCoffee ? null : <button onClick={() => {buyCoffee()}}> 커피 결제 </button>}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게도 가능합니다~ 가급적 요렇게 쓰시길 권장드려요

Suggested change
{!orderCoffee ? null : <button onClick={() => {buyCoffee()}}> 커피 결제 </button>}
{!orderCoffee ? null : <button onClick={buyCoffee}> 커피 결제 </button>}

);

}
export default App;
Copy link
Collaborator

Choose a reason for hiding this comment

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

EOF에 대해 알아보시고 없애주세욥

Copy link
Member

@ummaeha ummaeha left a comment

Choose a reason for hiding this comment

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

윤성님께서 리뷰 잘 해주셔서 추가 코멘트만 달았습니다.

다음 미션부터는 lint prettier 설정해주세요!
package-lock.json conflict 처리하고 바로 머지해주세요~

고생하셨습니다!


<div>
{!orderCoffee ? null : <button onClick={() => {buyCoffee()}}> 커피 결제 </button>}
{useCoupon ? <button onClick={() => {drinkCoffee()}}> 쿠폰 사용 </button> : null }
Copy link
Member

Choose a reason for hiding this comment

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

&& 연산자로 처리해도될 것 같습니다.


const buyCoffee = () => {
setCouponNum(couponNum+1);
if (couponNum === 10){
Copy link
Member

Choose a reason for hiding this comment

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

early return 가시죠!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mission 미션 입니다!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants