[1단계 - 계산기] 앨버(송상민) 미션 제출합니다. #27

al-bur · 2022-04-21T07:58:14Z

안녕하세요 로이정님 앨버입니다!

레벨2 첫 미션 잘부탁드리겠습니다~
리액트 경험이 많지 않지만 열심히 구현했습니다.

데모페이지

https://al-bur.github.io/react-calculator/

질문

1. JSX에서 null과 undefined

현재 total을 표시하는 JSX에서 내부 textNode로 첫째 항, 연산자, 둘째 항을 각각 출력하게 되어있습니다. 이 때 초기 상태에서는 연산자의 경우 null로 설정되어있는데, 이 부분이 null이라는 텍스트로 출력되는 것이 아니라 아예 무시되는 것을(아무것도 출력되지 않는 것을) 발견했습니다.

현재는 그 결과가 원하는 방식이라서 그 상태로 구현했는데 혹시 이것이 예상된 방법인지, 지양해야 하는 방법은 아닌지 궁금했습니다!

2. 자식 컴포넌트에서 부모 컴포넌트의 상태 변경하기

숫자버튼, 연산 버튼, 초기화 버튼을 각각 컴포넌트로 분리하면서 해당 자식 컴포넌트들에서 부모 컴포넌트인 계산기 컴포넌트의 상태를 변경해야 하는 상황이 발생했습니다. 그래서 선택한 방법은 부모 컴포넌트의 매서드로 setState를 감싼 뒤, 해당 매서드를 자식 컴포넌트의 prop으로 전달해서 접근, 사용할 수 있게 변경하는 방법이었습니다. 혹시 이 방식이 안티 패턴에 속하지 않을지 의문이 들었습니다.

3. 요소 배열의 key와 index

숫자 버튼의 경우 map으로 요소를 생성해서 배열에 담은 뒤 사용하고 있습니다. 이 생성 과정에서 요소가 없는 배열을 Array.from으로 만들어서 활용했는데, 문제는 이 배열은 값이 없기 때문에 index를 활용해서 key 값을 지정할 수 밖에 없었습니다.

그래서 검색해보던 중, 다음 조건들을 만족하는 경우 아주 예외적으로 index를 활용할 수도 있다는 글을 발견했습니다.

리스트와 아이템이 정적이고 계산된 결과가 아니며 불변
배열 안의 아이템에 고유한 id가 없음
절대 이 배열의 순서가 변경되거나 배열이 필터될 일이 없음

혹시 지금의 경우가 이 조건에 만족되는 지, 아니면 현재처럼 의미가 없는 배열을 통해서 요소를 생성하는 것 자체가 지양해야 하는 일인지 리뷰어님의 의견이 궁금했습니다!

Co-authored-by: uk960214 <[email protected]>

roy-jung

안녕하세요 엘버, 리뷰어 로이입니다.
리액트 하려니 낯설죠? 여기서는 간단히 질문에 대한 답변만 드려볼게요.

{null}이 null이라는 텍스트로 출력되는 것이 아니라 아예 무시되는 것이 예상된 방법인지, 지양해야 하는 방법인지

JSX를 처리함에 있어서 리액트가 상당히 똑똑하게 처리해줘서 고마운 부분 중 하나입니다. 지양하다뇨 당치않습니다!

자식 컴포넌트에서 부모 컴포넌트의 상태 변경하기

별도 코멘트로 남겼습니다.

index를 key 값으로 삼는 것이 안티패턴인지?
[배열 안의 아이템에 고유한 id가 없음. 절대 이 배열의 순서가 변경되거나 배열이 필터될 일이 없음]

괜찮습니다. 전혀 문제되지 않아요. index를 key로 삼는 것이 문제가 되는 경우는, 배열 내부 요소들의 순서가 변경되는 경우인데, 말씀하신 케이스는 순서가 바뀔 수가 없겠죠.

roy-jung · 2022-04-22T04:26:31Z

src/App.jsx

+    this.initialState = {
+      firstOperand: '0',
+      secondOperand: '',
+      operator: null,
+      isError: false,
+    };


initialState는 보통 외부변수로 작성합니다.

initialState도 클래스에 종속적인 데이터를 사용하지 않는 부분이라서 외부변수로 작성하는 것이군요!

수정하였습니다. 3663d1d

roy-jung · 2022-04-22T04:26:56Z

src/App.jsx

+    };
+
+    this.state = {
+      ...(JSON.parse(localStorage.getItem('state')) || this.initialState),


JSON.parse 결과를 다시 spread하신 특별한 이유가 있나요?

이 부분은 제가 대상을 잘못 지정해놓았네요! JSON.parse 결과가 아닌 this.initialState를 spread 할 목적이었습니다!

수정해주었습니다. 8284ced

roy-jung · 2022-04-22T04:28:45Z

src/App.jsx

+    window.addEventListener('beforeunload', this.#handleBeforeUnload);
+
+    window.addEventListener('pagehide', () => {
+      const { firstOperand, operator } = this.state;
+
+      if (firstOperand !== '0' || operator) {
+        window.localStorage.setItem('state', JSON.stringify(this.state));
+      }
+
+      window.removeEventListener('beforeunload', this.#handleBeforeUnload);
+    });


event 등록 대상이 window이니, 본 컴포넌트의 mount가 완료된 시점이 아니더라도 상관없을것 같군요.

로이님 말씀은 constructor에서 해줘도 되는 작업이라는 뜻일까요?

그렇다면, 불필요하게 lifecycle methods를 사용하는 것은 피하는 게 좋은 것일까요?

roy-jung · 2022-04-22T04:30:21Z

src/App.jsx

+          <DigitButtons state={this.state} handleParentState={this.#handleParentState} />
+          <AllClearButton handleParentState={this.#handleParentState} />
+          <OperationButtons
+            state={this.state}
+            initialState={this.initialState}
+            handleParentState={this.#handleParentState}


그냥 this.setState를 전달해도 되지 않나요?

state 및 setState를 그대로 하위 컴포넌트에 전달하는 방식은 안티패턴입니다. 하위 컴포넌트에는 각각 필요한 정보만 props로 전달해주세요. 예를 들어 DigitButtons에 필요한 값은 operator상태 및 operator값을 변경하는 메서드가 있겠죠. 이 때 메서드는 setState가 아니라 setOperand같은 별도의 메소드를 만들어 전달해주시는게 좋습니다. 그렇게 하면 하위 컴포넌트에서 상위컴포넌트의 상태명을 찾아 기재하지 않아도 되겠죠.
ex)

class App ... { ... setOperandValue = (operand, value) => { this.setState({ [operand === 'first' ? 'firstOperand' : 'secondOperand']: value }) } ... render() { return ( <DigitButtons operator={operator} operand={operand} setOperand={this.setOperandValue} /> ...

그대로 전달하는 것이 안티패턴이군요...왜 안티패턴인지에 대한 이유는 틈틈이 찾아보고 있습니다!!(아직 명확한 해답은 찾지 못했네요) 꼼꼼히 피드백 주셔서 감사합니다

수정하였습니다. d141c4b

roy-jung · 2022-04-22T04:44:15Z

src/components/AllClearButton.jsx

+  #handleAllClear = () => {
+    const { handleParentState } = this.props;
+    handleParentState({
+      firstOperand: '0',
+      secondOperand: '',
+      operator: null,
+      isError: false,
+    });
+  };


이 경우에도 handleAllClear라는 메소드를 상위에서 만들어서 전달해주는게 좋습니다.

수정하였습니다. d141c4b

roy-jung · 2022-04-22T06:03:04Z

src/components/OperationButtons.jsx

+    this.#calculation = {
+      '+': (a, b) => a + b,
+      '-': (a, b) => a - b,
+      X: (a, b) => a * b,
+      '/': (a, b) => Math.floor(a / b),
+    };


클래스에 종속적인 데이터를 사용하지 않는 이런 경우는 외부변수로 빼주시는게 좋아요.

소수점 이하는 버린다라고 하면 흔히 Math.floor를 떠올리게 되는데, 결과가 음수일 경우에도 고려해 보아야 합니다. 예를 들어

expression original result floor ceil round trunc

-13 / 4 -3.25 -4 -3 -3 -3

-14 / 4 -3.5 -4 -3 -4 -3

-15 / 4 -3.75 -4 -3 -4 -3

소수점 이하는 버린다의 조건에 부합하는 명령은 어떤 것일까요? 수학적으로 참인 것과 일반적인 인식이 일치하나요?

클래스에 종속적인 데이터를 사용하지 않는 이런 경우는 외부변수로 빼주시는게 좋아요.

혹시 왜 좋은지 이유를 알 수 있을까요?

제가 생각해본 이유는 클래스에 종속적인 데이터를 사용하는 변수/함수와 사용하지 않는 변수/함수를 나눠놓기 위해정도 밖에 생각하지 못했습니다.

수정하였습니다. be739cd

소수점 이하는 버린다의 조건에 부합하는 명령은 어떤 것일까요?

Math.floor() 함수는 주어진 숫자와 같거나 작은 정수 중에서 가장 큰 수를 반환합니다.
Math.trunc() 함수는 주어진 값의 소수부분을 제거하고 숫자의 정수부분을 반환합니다.

위 두 개를 비교해봤을때, 소수점 이하는 버린다의 조건에 부합하는 명령은 Math.trunc()이네요. 음수 부분을 신경쓰지 못했었네요.

수정하였습니다. dde1452

roy-jung · 2022-04-22T06:03:47Z

src/components/OperationButtons.jsx

+  }
+
+  #handleOperatorClick = ({ target }) => {
+    if (!target.classList.contains('operation')) return;


operator를 클릭했는데 operation 클래스가 없는 경우가 있나요?

다시 확인해보니 없는 경우는 없네요!! 생각해보니 지금 방법 (각 버튼에 이벤트 핸들러 달아주기) 이전에 부모 요소 하나에 이벤트 달아주고 이벤트 위임을 사용해줬었을때 사용했던 코드였던 것 같습니다.

꼼꼼하게 코드를 챙기는 습관을 들이겠습니다~ 73eca50

roy-jung · 2022-04-22T06:15:16Z

src/components/OperationButtons.jsx

+    if (!target.classList.contains('operation')) return;
+
+    const { secondOperand } = this.props.state;
+    if (secondOperand) return;


2번째 피연산자가 입력된 상태에서 다시 연산자를 누르면 아무일도 일어나지 않겠군요.
사용자 입장에서 아무일도 일어나지 않는 현상에 대해 괜찮다고 여길까요?

일반적인 계산기 프로그램은 연산자를 누르면 계산이 되는군요..

이 부분은 사용자들이 혼란이 올 수 있겠네요! 2단계 진행해보면서 고민해보겠습니다!

- 관련 피드백: woowacourse#27 (comment)

- Math.floor는 음수에서 기대한 바와 다르게 작동하여 Math.trunc 사용 관련 피드백: woowacourse#27 (comment)

관련 피드백: woowacourse#27 (comment)

- 관련 피드백: woowacourse#27 (comment)

- JSON.parse 한 값을 spread 할 필요가 없고 initalState를 spread 해줘야하므로 변경 관련 피드백: woowacourse#27 (comment)

- state와 setState를 있는 그대로 자식에게 내리는 것은 안티패턴이므로, 필요한 state를 추려 보내주고, 부모의 state를 변경하는 메서드를 부모에서 만들어서 따로 자식에게 내려주도록 변경 관련 피드백: woowacourse#27 (comment)

al-bur · 2022-04-24T05:50:06Z

안녕하세요 로이님 앨버입니다!! 남겨주신 피드백 너무 잘보았습니다.

고민해보고 공부할 것이 많았습니다ㅎㅎ이어서 2단계 진행해볼게요!

2단계가 함수형으로 전환하는 것이다 보니 1단계 피드백 수정 후 코드가 날라갈 것 같아서 따로 branch를 파놓았습니다.
1단계 피드백 수정한 branch에서 봐주시면 감사하겠습니다

- 2번째 피연산자가 입력된 상태에서 다시 연산자를 누르면 결과가 계산될 수 있도록 기능 구현 관련 피드백: woowacourse#27 (comment)

* refactor: 사용되지 않는 private 변수 제거 * refactor: 클래스에 종속적인 데이터를 사용하지 않는 부분 외부로 이동 - 관련 피드백: #27 (comment) * fix: 소수점 이하 버리는 기능 오류 - Math.floor는 음수에서 기대한 바와 다르게 작동하여 Math.trunc 사용 관련 피드백: #27 (comment) * refactor: 불필요한 로직 제거 관련 피드백: #27 (comment) * refactor: 클래스에 종속적인 데이터를 사용하지 않는 부분 외부로 이동 - 관련 피드백: #27 (comment) * refactor: 계산기 초기 값 설정 로직 변경 - JSON.parse 한 값을 spread 할 필요가 없고 initalState를 spread 해줘야하므로 변경 관련 피드백: #27 (comment) * refactor: 안티패턴 로직 수정 - state와 setState를 있는 그대로 자식에게 내리는 것은 안티패턴이므로, 필요한 state를 추려 보내주고, 부모의 state를 변경하는 메서드를 부모에서 만들어서 따로 자식에게 내려주도록 변경 관련 피드백: #27 (comment) * refactor: 클래스 -> 함수형 컴포넌트로 변경 * feat: 결과가 계산되는 상황 추가에 따른 기능 구현 - 2번째 피연산자가 입력된 상태에서 다시 연산자를 누르면 결과가 계산될 수 있도록 기능 구현 관련 피드백: #27 (comment) * chore: 기본적인 웹사이트 설정

al-bur and others added 15 commits April 19, 2022 14:45

docs: 요구사항 목록 작성

6f01992

Co-authored-by: uk960214 <[email protected]>

chore: 초기 환경설정 세팅

871f6c6

Co-authored-by: uk960214 <[email protected]>

feat: 마크업과 css 작성

1482be5

Co-authored-by: uk960214 <[email protected]>

feat: 숫자 입력 기능 구현

755b36e

Co-authored-by: uk960214 <[email protected]>

feat: 연산자 입력 및 연산처리 기능 구현

256eee4

Co-authored-by: uk960214 <[email protected]>

feat: 출력값 초기화 기능 구현

3ea297c

Co-authored-by: uk960214 <[email protected]>

feat: 오류 메시지 출력 기능 구현

a165c54

Co-authored-by: uk960214 <[email protected]>

feat: 상수화

b419b48

Co-authored-by: uk960214 <[email protected]>

feat: 화면 이탈시 상태 저장 및 불러오기

121346b

Co-authored-by: uk960214 <[email protected]>

feat: 이탈시 이벤트 변경

8f7f666

Co-authored-by: uk960214 <[email protected]>

chore: gitignore + eslint 설정

2ceff90

Co-authored-by: uk960214 <[email protected]>

refactor: 숫자 버튼 컴포넌트 분리

65fa228

Co-authored-by: uk960214 <[email protected]>

refactor: 컴포넌트 분리 및 리팩토링

aa1874d

Co-authored-by: uk960214 <[email protected]>

chore: eslint + prettier 설정 변경

e10a616

Co-authored-by: uk960214 <[email protected]>

refactor: 연산자 컴포넌트 리팩토링

1f11f0d

Co-authored-by: uk960214 <[email protected]>

pocojang requested a review from roy-jung April 21, 2022 10:42

pocojang assigned roy-jung Apr 21, 2022

roy-jung approved these changes Apr 22, 2022

View reviewed changes

roy-jung merged commit 714d736 into woowacourse:al-bur Apr 22, 2022

al-bur added a commit to al-bur/react-calculator that referenced this pull request Apr 23, 2022

refactor: 클래스에 종속적인 데이터를 사용하지 않는 부분 외부로 이동

be739cd

- 관련 피드백: woowacourse#27 (comment)

al-bur added a commit to al-bur/react-calculator that referenced this pull request Apr 23, 2022

fix: 소수점 이하 버리는 기능 오류

dde1452

- Math.floor는 음수에서 기대한 바와 다르게 작동하여 Math.trunc 사용 관련 피드백: woowacourse#27 (comment)

al-bur added a commit to al-bur/react-calculator that referenced this pull request Apr 23, 2022

refactor: 불필요한 로직 제거

73eca50

관련 피드백: woowacourse#27 (comment)

al-bur added a commit to al-bur/react-calculator that referenced this pull request Apr 23, 2022

refactor: 클래스에 종속적인 데이터를 사용하지 않는 부분 외부로 이동

3663d1d

- 관련 피드백: woowacourse#27 (comment)

This was referenced Apr 24, 2022

[2단계 - 계산기] 송상민(앨버) 미션 제출합니다. #52

Closed

[2단계 - 계산기] 앨버(송상민) 미션 제출합니다. #65

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 계산기] 앨버(송상민) 미션 제출합니다. #27

[1단계 - 계산기] 앨버(송상민) 미션 제출합니다. #27

al-bur commented Apr 21, 2022 •

edited

Loading

roy-jung left a comment

roy-jung Apr 22, 2022

al-bur Apr 23, 2022

roy-jung Apr 22, 2022

al-bur Apr 23, 2022 •

edited

Loading

roy-jung Apr 22, 2022

al-bur Apr 23, 2022

roy-jung Apr 22, 2022

al-bur Apr 24, 2022

roy-jung Apr 22, 2022

al-bur Apr 24, 2022

roy-jung Apr 22, 2022

al-bur Apr 23, 2022 •

edited

Loading

al-bur Apr 23, 2022 •

edited

Loading

roy-jung Apr 22, 2022

al-bur Apr 23, 2022

roy-jung Apr 22, 2022

al-bur Apr 24, 2022

al-bur commented Apr 24, 2022

[1단계 - 계산기] 앨버(송상민) 미션 제출합니다. #27

[1단계 - 계산기] 앨버(송상민) 미션 제출합니다. #27

Conversation

al-bur commented Apr 21, 2022 • edited Loading

데모페이지

질문

1. JSX에서 null과 undefined

2. 자식 컴포넌트에서 부모 컴포넌트의 상태 변경하기

3. 요소 배열의 key와 index

roy-jung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

al-bur Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

al-bur Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

al-bur Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

al-bur commented Apr 24, 2022

al-bur commented Apr 21, 2022 •

edited

Loading

al-bur Apr 23, 2022 •

edited

Loading

al-bur Apr 23, 2022 •

edited

Loading

al-bur Apr 23, 2022 •

edited

Loading