-
Notifications
You must be signed in to change notification settings - Fork 37
[1단계 - 계산기] 유세지(김용래) 미션 제출합니다. #17
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
Changes from all commits
3260e83
288d353
15a29c3
c391256
a59cd66
cb852d1
897d77d
a3069a3
a17367a
ad43c6e
ded57b2
ddfd8c1
708e14b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # See https://help.github.com/articles/ignoring-files/ for more about ignoring files. | ||
|
|
||
| # dependencies | ||
| /node_modules | ||
| /.pnp | ||
| .pnp.js | ||
|
|
||
| # testing | ||
| /coverage | ||
|
|
||
| # production | ||
| /build | ||
|
|
||
| # misc | ||
| .DS_Store | ||
| .env.local | ||
| .env.development.local | ||
| .env.test.local | ||
| .env.production.local | ||
|
|
||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ## 요구 사항 | ||
|
|
||
| - [x] Class Component를 사용합니다. | ||
| - [x] 레벨1을 참고하여 REQUIREMENTS.md에 요구 사항 도출 | ||
| - [x] 2개의 숫자에 대해 덧셈이 가능하다. | ||
| - [x] 2개의 숫자에 대해 뺄셈이 가능하다. | ||
| - [x] 2개의 숫자에 대해 곱셈이 가능하다. | ||
| - [x] 2개의 숫자에 대해 나눗셈이 가능하다. | ||
| - [x] AC(All Clear)버튼을 누르면 0으로 초기화 한다. | ||
| - [x] 계산 결과를 표현할 때 소수점 이하는 버림한다. | ||
| - [x] 초기 로딩시 이전 값이 Local Storage에 존재한다면 초기 값으로 적용한다. | ||
| - [x] 출력값 있는 상황에 사용자의 페이지 이탈시 confirm을 활용해 사용자의 이탈 여부를 확인한다. | ||
| - [x] 항상 사용자의 이탈시 마지막 출력값을 Local Storage에 저장한다. | ||
| - [x] 연산의 결과값이 Infinity일 경우 오류라는 문자열을 보여준다. (아이폰 참고) |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| { | ||
| "name": "react-calculator", | ||
| "version": "0.1.0", | ||
| "private": true, | ||
| "dependencies": { | ||
| "@testing-library/jest-dom": "^5.16.4", | ||
| "@testing-library/react": "^13.1.1", | ||
| "@testing-library/user-event": "^13.5.0", | ||
| "gh-pages": "^3.2.3", | ||
| "react": "^18.0.0", | ||
| "react-dom": "^18.0.0", | ||
| "react-scripts": "5.0.1", | ||
| "web-vitals": "^2.1.4" | ||
| }, | ||
| "scripts": { | ||
| "start": "react-scripts start", | ||
| "build": "react-scripts build", | ||
| "test": "react-scripts test", | ||
| "eject": "react-scripts eject", | ||
| "deploy": "gh-pages -d build", | ||
| "predeploy": "npm run build" | ||
| }, | ||
| "eslintConfig": { | ||
| "extends": [ | ||
| "react-app", | ||
| "react-app/jest" | ||
| ] | ||
| }, | ||
| "browserslist": { | ||
| "production": [ | ||
| ">0.2%", | ||
| "not dead", | ||
| "not op_mini all" | ||
| ], | ||
| "development": [ | ||
| "last 1 chrome version", | ||
| "last 1 firefox version", | ||
| "last 1 safari version" | ||
| ] | ||
| }, | ||
| "homepage": "https://usageness.github.io/react-calculator" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <link rel="icon" href="%PUBLIC_URL%/favicon.ico" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
| <meta name="theme-color" content="#000000" /> | ||
| <meta | ||
| name="description" | ||
| content="Web site created using create-react-app" | ||
| /> | ||
| <link rel="apple-touch-icon" href="%PUBLIC_URL%/logo192.png" /> | ||
| <!-- | ||
| manifest.json provides metadata used when your web app is installed on a | ||
| user's mobile device or desktop. See https://developers.google.com/web/fundamentals/web-app-manifest/ | ||
| --> | ||
| <link rel="manifest" href="%PUBLIC_URL%/manifest.json" /> | ||
| <!-- | ||
| Notice the use of %PUBLIC_URL% in the tags above. | ||
| It will be replaced with the URL of the `public` folder during the build. | ||
| Only files inside the `public` folder can be referenced from the HTML. | ||
| Unlike "/favicon.ico" or "favicon.ico", "%PUBLIC_URL%/favicon.ico" will | ||
| work correctly both with client-side routing and a non-root public URL. | ||
| Learn how to configure a non-root public URL by running `npm run build`. | ||
| --> | ||
| <title>React App</title> | ||
| </head> | ||
| <body> | ||
| <noscript>You need to enable JavaScript to run this app.</noscript> | ||
| <div id="root"></div> | ||
| <!-- | ||
| This HTML file is a template. | ||
| If you open it directly in the browser, you will see an empty page. | ||
| You can add webfonts, meta tags, or analytics to this file. | ||
| The build step will place the bundled scripts into the <body> tag. | ||
| To begin the development, run `npm start` or `yarn start`. | ||
| To create a production bundle, use `npm run build` or `yarn build`. | ||
| --> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| { | ||
| "short_name": "React App", | ||
| "name": "Create React App Sample", | ||
| "icons": [ | ||
| { | ||
| "src": "favicon.ico", | ||
| "sizes": "64x64 32x32 24x24 16x16", | ||
| "type": "image/x-icon" | ||
| }, | ||
| { | ||
| "src": "logo192.png", | ||
| "type": "image/png", | ||
| "sizes": "192x192" | ||
| }, | ||
| { | ||
| "src": "logo512.png", | ||
| "type": "image/png", | ||
| "sizes": "512x512" | ||
| } | ||
| ], | ||
| "start_url": ".", | ||
| "display": "standalone", | ||
| "theme_color": "#000000", | ||
| "background_color": "#ffffff" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # https://www.robotstxt.org/robotstxt.html | ||
| User-agent: * | ||
| Disallow: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import React, { Component } from "react"; | ||
| import DisplayResult from "./component/DisplayResult"; | ||
| import CalculatorButton from "./component/CalculatorButton"; | ||
|
|
||
| export default class App extends Component { | ||
| state = { | ||
| firstNumber: 0, | ||
| operator: null, | ||
| secondNumber: 0, | ||
| isFirstNumber: true, | ||
| result: 0, | ||
| }; | ||
|
|
||
| componentDidMount() { | ||
| const prevValue = localStorage.getItem("prevValue") || 0; | ||
| this.setFirstNumber(Number(prevValue)); | ||
| this.setResult(Number(prevValue)); | ||
|
|
||
| window.addEventListener("beforeunload", function (e) { | ||
| e.preventDefault(); | ||
| e.returnValue = ""; | ||
| }); | ||
| } | ||
|
|
||
| initState = () => { | ||
| this.setState({ | ||
| firstNumber: 0, | ||
| operator: null, | ||
| secondNumber: 0, | ||
| isFirstNumber: true, | ||
| result: 0, | ||
| }); | ||
| }; | ||
|
|
||
| setFirstNumber = (number) => { | ||
| this.setState( | ||
| { | ||
| firstNumber: Number(number), | ||
| }, | ||
| localStorage.setItem("prevValue", number) | ||
| ); | ||
| }; | ||
|
|
||
| setOperator = (operator) => { | ||
| this.setState({ operator }); | ||
| }; | ||
|
|
||
| setSecondNumber = (number) => { | ||
| this.setState( | ||
| { | ||
| secondNumber: Number(number), | ||
| }, | ||
| localStorage.setItem("prevValue", number) | ||
| ); | ||
| }; | ||
|
|
||
| setResult = (result) => { | ||
| this.setState({ result }); | ||
| }; | ||
|
|
||
| setIsFirstNumber = (isFirstNumber) => { | ||
| this.setState({ isFirstNumber }); | ||
| }; | ||
|
|
||
| calculate = () => { | ||
| const res = (() => { | ||
| switch (this.state.operator) { | ||
| case "+": | ||
| return this.add(); | ||
| case "-": | ||
| return this.sub(); | ||
| case "X": | ||
| return this.multiple(); | ||
| case "/": | ||
| return this.divide(); | ||
| default: | ||
| throw new Error("존재하지 않는 연산자입니다."); | ||
| } | ||
| })(); | ||
|
|
||
| this.setState({ result: res }, () => { | ||
| this.initState(); | ||
|
|
||
| if (res === Infinity || isNaN(res)) { | ||
| this.setFirstNumber(0); | ||
| this.setResult("오류"); | ||
| localStorage.setItem("prevValue", "오류"); | ||
| return; | ||
| } | ||
|
|
||
| this.setFirstNumber(res); | ||
| this.setResult(res); | ||
| localStorage.setItem("prevValue", res); | ||
| }); | ||
| }; | ||
|
Comment on lines
+65
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이곳에서 즉시실행함수를 사용해야하는 이유나 이점이 있나요?? switch 문 내부에서 break 대신 return을 사용하려고 즉시 실행 함수로 만들어주었습니다! 지금 불필요하게 setState를 하는듯 보여요. 계산된 값을 처음 setState({result: res}) -> initState -> setResult('오류') or setResult(res) 이렇게 로직이 진행됩니다. 계산된값의 검증을 하고 최종적으로 set하면 될꺼 같으면 init이 필요한지 잘 모르겠습니다 확실히 검증하기 전에 state를 set하는것은 불필요한 로직처럼 보이네요 😅 수정하겠습니다! setFirtNumber(res)의 함수에 localStorage.setItem이 있다보니 localStorage도 업데이트도 불필요하게 두번 이루어지고 있는듯합니다 급하게 만들다보니 state나 localStorage 업데이트를 중복해서 수행하는 로직이 곳곳에 나타나는 것 같아요 😥 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switch 문 내부에서 break 대신 return을 사용하려고 즉시 실행 함수로 만들어주었습니다! calculate를 호출할때마다 res 변수를 할당하는게 불필요한 반복적인 연산같고(굉장히 작은 비용이겠지만ㅎㅎ) 세부적으로 코드를 읽어야 해서, 따함수나 메소드를 따로 빼서 사용하면 좋을것 같아요. 세지님은 어떻게 생각하시나요?? |
||
|
|
||
| add() { | ||
| return this.state.firstNumber + this.state.secondNumber; | ||
| } | ||
|
|
||
| sub() { | ||
| return this.state.firstNumber - this.state.secondNumber; | ||
| } | ||
|
|
||
| divide() { | ||
| return Math.floor(this.state.firstNumber / this.state.secondNumber); | ||
| } | ||
|
|
||
| multiple() { | ||
| return this.state.firstNumber * this.state.secondNumber; | ||
| } | ||
|
|
||
| render() { | ||
| return ( | ||
| <> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fragment 제거해도 될듯 합니다:) |
||
| <div id="app"> | ||
| <div className="calculator"> | ||
| <DisplayResult result={this.state.result} /> | ||
| <CalculatorButton | ||
| setFirstNumber={this.setFirstNumber} | ||
| setOperator={this.setOperator} | ||
| setSecondNumber={this.setSecondNumber} | ||
| setIsFirstNumber={this.setIsFirstNumber} | ||
| calculate={this.calculate} | ||
| setResult={this.setResult} | ||
| result={this.state.result} | ||
| initState={this.initState} | ||
| isFirstNumber={this.state.isFirstNumber} | ||
| firstNumber={this.state.firstNumber} | ||
| secondNumber={this.state.secondNumber} | ||
| /> | ||
|
Comment on lines
+119
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금 미션의 취지와 규모에서 재사용할일이 없긴하겠지만... 재사용하기가 정말 어려운 컴포넌트인것 같아요. 이 컴포넌트를 재사용한다고 했을때 어떤 이벤트에 로직을 주입하는것인지 파악하기 어렵습니다. 만약 AC를 클릭했을때 alert를 하도록 로직을 추가한다고 하면 어떻게 될까요? 새로 props도 뚫어줘야하고 컴포넌트 내부도 바뀌게 될것 같습니다
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 생각하기에도 자식 컴포넌트에 너무 많은 함수를 props로 전달해줘서 로직을 파악하기 어렵고, |
||
| </div> | ||
| </div> | ||
| </> | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import React, { Component } from "react"; | ||
|
|
||
| export default class CalculatorButton extends Component { | ||
| onClickNumber = (e) => { | ||
| const inputNumber = e.target.textContent; | ||
| if (this.props.result === 0) { | ||
| this.props.setResult(inputNumber); | ||
| } else { | ||
| this.props.setResult(this.props.result + inputNumber); | ||
| } | ||
|
|
||
| if (this.props.isFirstNumber) { | ||
| this.props.setFirstNumber( | ||
| this.props.firstNumber * 10 + Number(inputNumber) | ||
| ); | ||
| return; | ||
| } | ||
| this.props.setSecondNumber(this.props.secondNumber + inputNumber); | ||
| }; | ||
|
|
||
| onClickOperator = (e) => { | ||
| const inputOperator = e.target.textContent; | ||
| if (this.props.firstNumber === "") return; | ||
| if (inputOperator === "=" && this.props.secondNumber === "") return; | ||
| this.props.setResult(this.props.result + inputOperator); | ||
| if (inputOperator !== "=") { | ||
| this.props.setFirstNumber(this.props.firstNumber); | ||
| this.props.setOperator(inputOperator); | ||
| this.props.setIsFirstNumber(false); | ||
| return; | ||
| } | ||
|
|
||
| this.props.calculate(); | ||
| }; | ||
|
|
||
| onClickModifier = () => { | ||
| this.props.initState(); | ||
| localStorage.setItem("prevValue", 0); | ||
| }; | ||
|
|
||
| render() { | ||
| return ( | ||
| <> | ||
| <div className="digits flex" onClick={this.onClickNumber}> | ||
| <button className="digit">9</button> | ||
| <button className="digit">8</button> | ||
| <button className="digit">7</button> | ||
| <button className="digit">6</button> | ||
| <button className="digit">5</button> | ||
| <button className="digit">4</button> | ||
| <button className="digit">3</button> | ||
| <button className="digit">2</button> | ||
| <button className="digit">1</button> | ||
| <button className="digit">0</button> | ||
| </div> | ||
| <div className="modifiers subgrid" onClick={this.onClickModifier}> | ||
| <button className="modifier">AC</button> | ||
| </div> | ||
| <div className="operations subgrid" onClick={this.onClickOperator}> | ||
| <button className="operation">/</button> | ||
| <button className="operation">X</button> | ||
| <button className="operation">-</button> | ||
| <button className="operation">+</button> | ||
| <button className="operation">=</button> | ||
| </div> | ||
| </> | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import React, { Component } from "react"; | ||
|
|
||
| export default class DisplayResult extends Component { | ||
| render() { | ||
| return <h1 id="total">{this.props.result.toString()}</h1>; | ||
| } | ||
| } |
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.
setState를 wrapping 함수가 필요한가 했을때 저는 조금 의문이 들었습니다. 지금 규모에서 setState(인자)로도 의미전달은 충분할것 같고, 반복이 심하지 않고요.
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.
그렇다면 현재 구조에서 부모의 setState를 원형 그대로 자식 컴포넌트로 전달해주는 방법도 괜찮을까요?
자식 컴포넌트에게 너무 큰 권한을 가진 함수를 전달해주는 느낌이라 괜히 걱정이 되네요... 😥
Uh oh!
There was an error while loading. Please reload this page.
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.
밑에 calculatorbutton 컴포넌트와 같이 얘기할껄 그랬네요. 세지님이 말씀하신것처럼 setState를 자식컴포넌트에게 전달해주는건 저도 반대입니다. 지금의 컴포넌트 props를 유지한다면 해당 메소드들이 필요하겠죠.
제가 말씀드리려고 했던 부분은 크게 두가지였어요.