[holee] typescript-calculator by hochan222 · Pull Request #2 · transcendence42/javascript-calculator

hochan222 · 2021-06-01T13:01:37Z

https://hochan222.github.io/javascript-calculator/

BDD를 작성하면서, given, when, then 형식에 맞춰 테스트 케이스를 작성하려고했고, 전체 큰틀을 먼저정하고 각 섹션마다 테스트 케이스를 BDD 주기를 돌리면서 점점 세분화시켜가면서 구현했습니다. 코드를 짜기에 앞서서 먼저 계획을 세우고, 알맞은 동작을 먼저 정의해서 정해놓은 범위 안에서 기능들을 구현하는데 요구사항을 고민할 필요가 없어서 수월했습니다.

또한, 테스트 코드도 행동 중심으로 기술했기 때문에 프로그래밍을 모르는 사람도 테스트 코드가 어떤 동작를 테스트하는 코드인지 알 수 있다는 점이 BDD의 강점이라고 생각합니다.

그 외
- 타입스크립트를 처음 사용해보았는데 null과 any를 없애려고 노력했습니다.
- 정규표현식을 사용해서 전체 코드량을 줄였습니다.

…igit when calculate +

jwon42

데이터속성을 이용한 값 체크와 정규표현식을 이용한 검증 로직이 인상깊었고, 과제 진행 중 해당 부분을 차용했습니다.
cypress에서 파트들을 describe로 나누고 세부적인 것들을 it로 나눠야 테스트 결과 보기가 더 좋다는걸 알았네요.
전반적으로 가독성 좋고 깔끔한 코드 덕에 많이 배웠습니다 :)

jwon42 · 2021-06-04T04:08:09Z

src/js/calculate-util.ts

+  if (operator === '+') result = total + num;
+  if (operator === '/') result = total / num;
+  if (operator === '%') result = total % num;
+  if (operator === '-') result = total - num;
+  if (operator === 'X') result = total * num;


다양한 방법을 찾아보고 고민하다가 웹서버 메소드 나눌 때 생각이 나서 switch문을 사용해봤습니다.
https://github.com/transcendence42/javascript-calculator/pull/4/files#diff-2a93db0cc8b6388990eb7aa5c3a7ac60f207c94775e82ade82ec3503d0507c77

궁금해서 검색해보니 if문과 switch문이 속도에서 유의미한 차이는 없는 듯 합니다.

저도 어셈블리 생각나서 조금 찾아보았습니다.

int main() { int x=0; int y; if(x==0) y = 0; else if(x==1) y = 1; else if(x==2) y = 2; else if(x==5) y = 5; else if(x==6) y = 6; switch(x) { case 0 : y = 0; break; case 1 : y = 1; break; case 4 : y = 4; break; case 5 : y = 5; break; case 6 : y = 6; break; } return 0; }

을 어셈블리로 바꾸면

7: if(x==0) y = 0; 0040D4BF cmp dword ptr [ebp-4],0 0040D4C3 jne main+2Eh (0040d4ce) 0040D4C5 mov dword ptr [ebp-8],0 8: else if(x==1) y = 1; 0040D4CC jmp main+68h (0040d508) 0040D4CE cmp dword ptr [ebp-4],1 0040D4D2 jne main+3Dh (0040d4dd) 0040D4D4 mov dword ptr [ebp-8],1 9: else if(x==2) y = 2; 0040D4DB jmp main+68h (0040d508) 0040D4DD cmp dword ptr [ebp-4],2 0040D4E1 jne main+4Ch (0040d4ec) 0040D4E3 mov dword ptr [ebp-8],2 10: else if(x==5) y = 5; 0040D4EA jmp main+68h (0040d508) 0040D4EC cmp dword ptr [ebp-4],5 0040D4F0 jne main+5Bh (0040d4fb) 0040D4F2 mov dword ptr [ebp-8],5 11: else if(x==6) y = 6; 0040D4F9 jmp main+68h (0040d508) 0040D4FB cmp dword ptr [ebp-4],6 0040D4FF jne main+68h (0040d508) 0040D501 mov dword ptr [ebp-8],6 12: 13: 14: switch(x) 15: { 0040D508 mov eax,dword ptr [ebp-4] 0040D50B mov dword ptr [ebp-0Ch],eax 0040D50E cmp dword ptr [ebp-0Ch],6 0040D512 ja $L286+7 (0040d549) 0040D514 mov ecx,dword ptr [ebp-0Ch] 0040D517 jmp dword ptr [ecx*4+40D552h] 16: case 0 : y = 0; break; 0040D51E mov dword ptr [ebp-8],0 0040D525 jmp $L286+7 (0040d549) 17: case 1 : y = 1; break; 0040D527 mov dword ptr [ebp-8],1 0040D52E jmp $L286+7 (0040d549) 18: case 4 : y = 4; break; 0040D530 mov dword ptr [ebp-8],4 0040D537 jmp $L286+7 (0040d549) 19: case 5 : y = 5; break; 0040D539 mov dword ptr [ebp-8],5 0040D540 jmp $L286+7 (0040d549) 20: case 6 : y = 6; break; 0040D542 mov dword ptr [ebp-8],6 21: 22: }

라고 하는데요. 다르게 변환되는걸 볼 수 있습니다. switch문의 경우 hash table 혹은 배열로 만들어진 jump table로 구현이 된다고 합니다.
조건이 적을때는 if문이 더 좋을 수 있지만, 조건이 많아지면 switch문이 더 효율적입니다. 물론, 가독성이 더 중요하겠지만요..! 아래는 최적화에 대한 좋은 글 있어서 링크올렸습니다.

https://12bme.tistory.com/134

jwon42 · 2021-06-04T04:10:22Z

dist/js/controller.js

+    if (Number(totalTarget.innerText) === 0) {
+        return;
+    }
+    else if (!['/', '-', 'X', '+'].includes(totalTarget.innerText[totalTarget.innerText.length - 1])) {


이 부분도 정규표현식을 이용할 수 있지 않을까요?

오.. 맞아요 test 문법을 사용하면 간단하게 표현가능할 수 있을것 같아요!

jwon42 · 2021-06-04T04:12:11Z

src/js/calculate-util.ts

+
+const calculateResult = (total: string): number => {
+  const result: RegExpMatchArray | null = total.match(
+    new RegExp('(\\-?[\\d]{1,3})(X|\\-|\\+|\\/|\\=)(\\-?[\\d]{1,3})')


가운데 연산자를 추출하기 위한 정규표현식에서 =는 필요 없어 보입니다.

(과제 진행하며 정규표현식 사용에 대해 많은 도움을 받았습니다 👍)

맞네요! ㅎㅎ 오.. 감사합니다 :)

yhshin0

잘 봤습니다. 테스트도 함수로 빼내어 깔끔하게 작성하신 것이 좋았습니다.

수고하셨습니다!

yhshin0 · 2021-06-04T04:14:07Z

cypress/integration/counter.spec.js

+const testInputClickEvent = (first, result) => {
+  for (let item of first) {
+    cy.get('.digit')
+      .contains(item)
+      .click();
+  }
+  cy.get('#total').should('have.text', result);
+};


함수로 분리하니까 훨씬 깔끔하고 좋네요

yhshin0 · 2021-06-04T04:20:03Z

index.html

        <meta name="viewport" content="width=device-width, initial-scale=1.0" />
        <title>Calculator</title>
-        <link rel="stylesheet" type="text/css" href="src/css/index.css" />
+        <link rel="stylesheet" type="text/css" href="dist/css/index.css" />


dist 라는 이름의 의미는 뭔가요?

타입스크립트로 작성한 파일이 js로 변환되어 저장되는 폴더입니다.
tsconfig.json파일의 "outDir" 에서 확인 가능합니다.
https://github.com/transcendence42/javascript-calculator/pull/2/files#diff-b55cdbef4907b7045f32cc5360d48d262cca5f94062e353089f189f4460039e0

dist는 distribute의 약자로 배포의 의미를 가지고 있습니다. src에는 코드 작성부인 ts 파일들이있고 js파일들을 tsconfig를 통해서 dist로 분리시켰습니다!

yhshin0 · 2021-06-04T04:25:50Z

dist/js/calculate-util.js

+    if (operator === '%')
+        result = total % num;


프로그램에서 % 연산이 쓰이는 것 같진 않네요

앗앗 맞아요 ㅎㅎ 예전에 쓰던 함수를 그대로 긁어와서 레거시가 생겼네요..!

yhshin0 · 2021-06-04T04:26:40Z

src/js/operation-data-set.ts

+const getCountOperationdataSet = (): string => {
+  const operations = document.getElementsByClassName(
+    'operations'
+  )[0]! as HTMLElement;


HTMLDivElement로 바꿔도 될 것 같습니다

저도 동의합니다. 타입스크립트로 작성하는 만큼 조금 더 구체적 타입을 지정해야한다고 생각합니다.

yhshin0 · 2021-06-04T04:44:19Z

src/js/controller.ts

+const ACEvent = (): void => {
+  document.getElementById('total')!.innerText = '0';
+};


AC를 클릭할 때 operations data-count도 초기화를 하는 것이 좋을것 같아요(setCountOperationdataSet('0');)

예를 들어, 6+4를 누르고 AC를 누른 다음, 5+3을 누르려고 하면 +가 나타나지 않습니다

오 좋은 피드백이네요

오.. 감사합니다!

yechoi42

정규표현식을 사용해서 코드를 짠 부분이 깔끔했어요! 테스트 코드도 기능별로 잘 쪼갠 것 같고요.
하나 제안 사항은 결과값이 3자리를 넘어서면, 3자리로 잘라주는데 그 타이밍을 좀 일찍 잡아주면 더 좋을 것 같아요.

예를 들면 123*123 = 15129 결과값을 받고 나서
+ 를 입력하면 여기까지 그대로 잘 입력되고
그다음에 숫자를 입력하면 151로 잘리는데,

+를 입력할때 15129가 151로 잘리고 + 를 보여주면 더 자연스러울 것 같아요 !

yechoi42 · 2021-06-04T05:03:14Z

src/js/controller.ts

+  const eventTarget: HTMLElement = e.target as HTMLElement;
+  const totalTarget: HTMLElement | null = document.getElementById('total');


eventTarget은 타입단언을 해줬는데 totalTarget은 하지 않은 이유가 혹시 있을까요?

처음 쓰다보니 에러로 안잡아줘서 잘 몰랐습니다.

const totalTarget: HTMLHeadingElement | null = document.getElementById('total') as HTMLHeadingElement;

지금은 위와같이 구체화 시키는게 좋다고 생각합니다.

yechoi42 · 2021-06-04T05:10:39Z

src/js/check.ts

+  if (!result) {
+    return '';
+  }
+  if (result[2] === undefined) {
+    if (result[1] === undefined) {
+
+      return '';
+    }
+
+    return result[1];
+  }
+  if (result[3] === undefined) {
+
+    return result[1] + result[2];
+  }


Suggested change

if (!result) {

return '';

}

if (result[2] === undefined) {

if (result[1] === undefined) {

return '';

}

return result[1];

}

if (result[3] === undefined) {

return result[1] + result[2];

}

if (!result) {

return '';

}

else if (result[2] === undefined) {

if (result[1] === undefined) {

return '';

}

return result[1];

}

else if (result[3] === undefined) {

return result[1] + result[2];

}

yechoi42 · 2021-06-04T05:11:23Z

src/js/check.ts

@@ -0,0 +1,22 @@
+export const checkValidInput = (total: string): string => {


이 함수 깔끔하네요 !

yechoi42 · 2021-06-04T05:24:53Z

src/js/operation-data-set.ts

+const setCountOperationdataSet = (count: string): void => {
+  const operations = document.getElementsByClassName(
+    'operations'
+  )[0]! as HTMLElement;
+  operations.dataset['count'] = count;
+};


여기서 사용하는 플래그가 스트링 자료형이니까, '0', '1'보단 기왕이면 의미를 담은 문자열이면 더 좋을 것 같아요 !

동의합니다!

yechoi42 · 2021-06-04T05:25:14Z

src/js/controller.ts

+const ACEvent = (): void => {
+  document.getElementById('total')!.innerText = '0';
+};


오 좋은 피드백이네요

hochan222 added 25 commits June 1, 2021 15:17

chore: gitignore, prettierc, tsconfing, package.json, cypress

2b4c4cc

docs: cypress examples 둘러보기 완료

a7106a8

chore: config typescript setting

22e4a32

tsconfig.json: Build:No inputs were found in config file

13f5f8d

feat: add initial value & Render digit when button clicked & Render d…

e4cd2a7

…igit when calculate +

chore: typescript를 시작하기위한 기본 설정 설정 완료

3c7d913

test: 0 부터 9까지 버튼 클릭 후 값출력 완료

62f2112

test: make case: Max input length 3

5269a22

feat: add Max input length 3

2e706bd

refactor: testcase 함수로 묶어서 리펙토링함

517bbe2

test: add case Calculate two input when button click

8f301d1

feat: add operator

aaf5cbb

feat: add Calculate two input when button click

e0c8df6

refactor: test case, = 버튼도 눌르게 변경

9207148

feat: 연산자 기능 구현 완료

185e02f

feat: 계산 결과를 표현할 때 소수점 이하는 버림한다.

85bf0ed

chore: add test case 계산 결과를 표현할 때 소수점 이하는 버림한다.

1ab968c

feat: AC(All Clear) 버튼을 누를때 total을 0으로 변경한다.

2a78429

feat: operator가 연속으로 나왔을때 한개만 적용되도록 처리

33ea372

feat: operator가 2번이상 나왔을때 한개만 적용되도록 처리

d00ae27

refactor: 함수 파일별로 나누기

6b7a4c5

feat: add init func

b07c74b

feat: 정규표현시 할당 된 변수 타입 구체적으로 변경

91944e8

feat: 정규표현시 할당 된 변수 타입 구체적으로 변경

041dfb5

fix: Uncaught TypeError: Cannot read property '1' of null(…)

04b7bbe

jwon42 approved these changes Jun 4, 2021

View reviewed changes

yhshin0 reviewed Jun 4, 2021

View reviewed changes

yechoi42 approved these changes Jun 4, 2021

View reviewed changes

hochan222 changed the title ~~[holee] javascript-calculator~~ [holee] typescript-calculator Jun 5, 2021

		const eventTarget: HTMLElement = e.target as HTMLElement;
		const totalTarget: HTMLElement \| null = document.getElementById('total');

		@@ -0,0 +1,22 @@
		export const checkValidInput = (total: string): string => {

Conversation

hochan222 commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwon42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yhshin0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yechoi42 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hochan222 commented Jun 1, 2021 •

edited

Loading

yechoi42 left a comment •

edited

Loading