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

[1단계 - 계산기] 밧드(감우영) 미션 제출합니다. #12

Merged
merged 12 commits into from
Apr 21, 2022

Conversation

kamwoo
Copy link

@kamwoo kamwoo commented Apr 21, 2022

데모페이지
계산기 미션 제출합니다.

🎯 기능 요구사항

  • 2개의 숫자에 대해 덧셈이 가능하다.
  • 2개의 숫자에 대해 뺄셈이 가능하다.
  • 2개의 숫자에 대해 곱셈이 가능하다.
  • 2개의 숫자에 대해 나눗셈이 가능하다.
  • AC(All Clear)버튼을 누르면 0으로 초기화 한다.
  • 숫자는 한번에 최대 3자리 수까지 입력 가능하다.
  • 계산 결과를 표현할 때 소수점 이하는 버림한다.

필수 요구사항

  • Class Component를 사용합니다.
  • 초기 로딩시 이전 값이 Local Storage에 존재한다면 초기 값으로 적용한다.
  • 출력값 있는 상황에 사용자의 페이지 이탈시 confirm을 활용해 사용자의 이탈 여부를 확인한다.
  • 항상 사용자의 이탈시 마지막 출력값을 Local Storage에 저장한다.
  • 연산의 결과값이 Infinity일 경우 오류라는 문자열을 보여준다.

kamwoo and others added 12 commits April 19, 2022 14:54
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Co-authored-by: JASUN LEE <liswktjs@users.noreply.github.com>
Copy link

@HyeonaKwon HyeonaKwon left a comment

Choose a reason for hiding this comment

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

안녕하세요 밧드! 이번 계산기 미션의 리뷰어 헤인티입니다. 만나게 되어 반갑습니다 :)

전체적으로 코드가 깔끔하네요~! 간단하게 피드백 남겼으니 확인해봐주시면 됩니다!
1단계 고생하셨구 2단계에서 봬요!


### `npm run build` fails to minify

This section has moved here: [https://facebook.github.io/create-react-app/docs/troubleshooting#npm-run-build-fails-to-minify](https://facebook.github.io/create-react-app/docs/troubleshooting#npm-run-build-fails-to-minify)

Choose a reason for hiding this comment

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

리드미는 정리하면 좋겠네요!

"last 1 safari version"
]
}
}

Choose a reason for hiding this comment

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

프리티어, 린트 패키지들이 추가되면 좋겠네요!

@@ -0,0 +1,17 @@
import React, { Component } from 'react';

Choose a reason for hiding this comment

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

react 18 에서는 import React from 'react' 생략이 가능합니다!

Suggested change
import React, { Component } from 'react';
import { Component } from 'react';

class Calculator extends Component {
constructor() {
super();
window.addEventListener('beforeunload', this.confirmExist);

Choose a reason for hiding this comment

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

이 리스너는 언제 해제될까요?

Comment on lines +21 to +23
prevNumber: [],
operator: '',
nextNumber: [],

Choose a reason for hiding this comment

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

prevNumber -> prevNumbers , nextNumber -> nextNumbers
복수 표현해주세요~

Comment on lines +45 to +59
try {
if (isPrev) {
if (this.state.prevNumber.length >= NUMBER_LIMIT) {
throw new Error(ERROR_MSG.OVER_NUMBER_LIMIT);
}
this.setState({ prevNumber: [...this.state.prevNumber, number] });
return;
}
if (this.state.nextNumber.length >= NUMBER_LIMIT) {
throw new Error(ERROR_MSG.OVER_NUMBER_LIMIT);
}
this.setState({ nextNumber: [...this.state.nextNumber, number] });
} catch ({ message }) {
alert(message);
}

Choose a reason for hiding this comment

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

불필요한 try-catch 와 같이 댑스가 깊어지는 일은 피해주세요~!

Suggested change
try {
if (isPrev) {
if (this.state.prevNumber.length >= NUMBER_LIMIT) {
throw new Error(ERROR_MSG.OVER_NUMBER_LIMIT);
}
this.setState({ prevNumber: [...this.state.prevNumber, number] });
return;
}
if (this.state.nextNumber.length >= NUMBER_LIMIT) {
throw new Error(ERROR_MSG.OVER_NUMBER_LIMIT);
}
this.setState({ nextNumber: [...this.state.nextNumber, number] });
} catch ({ message }) {
alert(message);
}
const { prevNumber, nextNumber } = this.state;
if (prevNumber.length >= NUMBER_LIMIT || nextNumber.length >= NUMBER_LIMIT) {
alert(ERROR_MSG.OVER_NUMBER_LIMIT);
return;
}
if (isPrev) {
this.setState({ prevNumber: [...this.state.prevNumber, number] });
return;
}
this.setState({ prevNumber: [...this.state.prevNumber, number] });

Comment on lines +105 to +107
{Array.from({ length: 10 }).map((_, index) => (
<NumberButtons key={index} func={this.onClickNumber} number={-(index - 9)} />
))}

Choose a reason for hiding this comment

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

👍

@HyeonaKwon HyeonaKwon merged commit 0d2fa8b into woowacourse:kamwoo Apr 21, 2022
@kamwoo kamwoo deleted the step1 branch April 22, 2022 04:29
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.

None yet

2 participants