[yechoi] typescript-calculator #1

yechoi42 · 2021-05-31T07:51:34Z

TDD 방식을 처음으로 경험해보았습니다.
코드를 수정하면서 원래 됐던 케이스가 안되기도 했는데
이런 걸 쉽게 확인할 수 있어서 유용했어요.

그리고 처음부터 제대로된 테스트 코드를 짜는 것이 생각보다 어렵더라고요.
놓친 테스트 케이스가 있어서 중간에 추가하기도 했고,
테스트 코드에 '='을 넣는 걸 깜빡해서 나중에 한꺼번에 추가하기도 했습니다.
코드를 작성하기 이전에 테스트 케이스를 명확히하고, 테스트 코드 구현도 섬세히 해야겠습니다.

입력받을 때 초기화 여부판단 기준으로 삼음

hochan222

parsedInput class 및 다른 함수들도 기능별로 잘 나눠져있어서 코드를 읽는데 좋았습니다. 코드가 굉장히 깔끔하네요! 많이 배워가요! 고생하셨습니다 :)

hochan222 · 2021-06-04T03:32:33Z

.eslintrc.json

+  "extends": [
+    "plugin:cypress/recommended"
+  ]
+}


EOL 아티클 읽어보시면 좋을것 같아요!

hochan222 · 2021-06-04T03:40:58Z

src/ts/index.ts

+  num2: string;
+  operator: string;
+
+  constructor(operator: string, num1: string, num2: string) {


Suggested change

constructor(operator: string, num1: string, num2: string) {

constructor(operator: string, num1: string, num2: string):void {

typescript인 만큼 void일때도 명시해주는게 가독성을 위해 좋다고 생각합니다.

hochan222 · 2021-06-04T03:44:08Z

src/ts/index.ts

+  const total = document.getElementById("total") as HTMLElement;
+  let prompt = total.innerHTML as string;


Suggested change

const total = document.getElementById("total") as HTMLElement;

let prompt = total.innerHTML as string;

const total: HTMLElement = document.getElementById("total") as HTMLElement;

let prompt: string = total.innerHTML as string;

저는 적는게 좋다고 생각하는데 혹시 어떻게 생각하시나요?

저도 빠뜨린 부분이 있는데 적어주는게 좋다고 생각합니다.

@hochan222 @jwon42 뒤에 강제 타입캐스팅을 해놓더라도 가독성 측면에서 앞에 있는 게 좋다는 의미죠?

@yechoi42 둘이 다른 의미라고 생각합니다. c 언어로 치자면

HTMLElement total = (HTMLElement)document.getElementById("total");

따라서, total 뒤에 붙는 HTMLElement는 형변환 의미가 아닌 변수 선언의 Type의미를 가져서 생략하면 안된다고 생각합니다.

hochan222 · 2021-06-04T03:45:32Z

src/ts/index.ts

+}
+
+function showInput(str: string) {
+  const prompt= document.getElementById("total") as HTMLElement;


Suggested change

const prompt= document.getElementById("total") as HTMLElement;

const prompt = document.getElementById("total") as HTMLElement;

hochan222 · 2021-06-04T03:46:49Z

src/ts/index.ts

+}
+
+function setOperationsController() {
+  const operations = document.getElementsByClassName("operations");


이부분도 타입 명시를 해주면 좋을것 같아요!

hochan222 · 2021-06-04T03:51:50Z

src/ts/index.ts

+  const oldText = prompt.innerHTML;
+  if (prompt.dataset.type === "result") {
+    console.log(str);
+    prompt.innerHTML = str;


Suggested change

prompt.innerHTML = str;

prompt.innerHTML = str;

innerHTML은 비용이 굉장히 큰 함수라서 (하위 돔 파괴 생성 및 이벤트까지 없애줌) text만 다루는거면 innerText를 사용하는게 좋아보여요!

yhshin0

과제 요구사항을 잘 만족하셨습니다. cypress.json 파일을 활용한 것도 인상적이었습니다.

다만 음수가 첫번째 숫자로 들어오는 경우, 두번째 숫자가 세자리를 넘길 수 있는게 조금 아쉽네요.

수고하셨습니다!

yhshin0 · 2021-06-04T03:27:21Z

src/ts/index.ts

+}
+
+function checkInputValidation(input: string, str: string): boolean {
+  let newStr: string = str[0] === "-" ? str.slice(1, prompt.length) : str;


여기 prompt는 어떤 역할인가요?

total 창에 띄워진 문자열인데요, 스코프를 적절하지 않게 사용한 것 같네요. 보완해야 할 부분인 것 같습니다.

yhshin0 · 2021-06-04T03:42:50Z

cypress/integration/calculator.ts

+    cy.get("#total").should("have.text", 9 + 2);
+  });
+  it("add one digit numbers: -9 + 2", () => {
+    cy.reload();


reload는 새로고침인가보네요. 하나 배워갑니다 :)

yhshin0 · 2021-06-04T03:53:22Z

index.html

            </div>
        </div>
    </body>
+    <script type="module" src="src/ts/index.js"></script>


tsconfig.json에서는 컴파일된 js 파일이 src/js/ 경로에 위치하는데 index.html에서는 src/ts/ 경로의 파일을 참조하고 있네요

yhshin0 · 2021-06-04T03:54:56Z

src/ts/index.ts

+
+function parseInput(str: string): parsedInput {
+  const total = document.getElementById("total") as HTMLElement;
+  let prompt = total.innerHTML as string;


이 프로그램에서 window.prompt() 함수를 사용하진 않지만, prompt 보다는 다른 변수명을 사용하는 것이 좋아 보입니다.

예약어 느낌이군요. 저도 배워갑니다.

jwon42

데이터 속성을 사용해서 result를 체크하는게 좋았습니다.

우린 그냥 c언어도 아닌 42의 많은 제약이 있는 c언어로 코딩을 시작해서 그런지 무언가 필요하면 만들고 보는 습관이 있는데 자바스크립트에선 대부분의 것들이 구현되어있다는 가정 하에 검색을 많이 해봐야 할 것 같아요.

수고하셨습니다 :)

jwon42 · 2021-06-04T04:41:16Z

src/ts/index.ts

+  const total = document.getElementById("total") as HTMLElement;
+  let prompt = total.innerHTML as string;


저도 빠뜨린 부분이 있는데 적어주는게 좋다고 생각합니다.

jwon42 · 2021-06-04T04:56:55Z

src/ts/index.ts

+  }
+}
+
+function findOperator(str: string): string {


string includes 메서드 혹은 정규표현식의 test 혹은 search를 이용하면 더 간단하게 연산자를 추출할 수 있을 듯 합니다.
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/includes

jwon42 · 2021-06-04T05:14:55Z

src/ts/index.ts

+function isOperator(input: string): boolean {
+  switch (input) {
+    case "/":
+      return true;
+    case "X":
+      return true;
+    case "-":
+      return true;
+    case "+":
+      return true;
+    default:
+      return false;
+  }
+}


이 부분도 정규표현식 혹은 string includes를 이용하면 더 간결할 듯 합니다!

jwon42 · 2021-06-04T05:16:02Z

src/ts/index.ts

+    operator === "none" ? [prompt, "1"] : prompt.split(operator);
+  return new parsedInput(
+    operator,
+    startWithMinus ? "-" + nums[0] : nums[0],


- 부호를 위한 예외처리군요. 플래그체크와 삼항연산자라니 c언어를 보는 것 같습니다 😁

jwon42 · 2021-06-04T05:17:45Z

src/ts/index.ts

+function showInput(str: string) {
+  const prompt= document.getElementById("total") as HTMLElement;
+  const oldText = prompt.innerHTML;
+  if (prompt.dataset.type === "result") {


적절한 곳에 데이터속성 사용 좋네요.

yechoi42 added 20 commits May 31, 2021 16:48

chore: install cypress

8ab7b67

feat: init calculator project'

e3f2fbb

test. add addition test

3058c4b

test: add substraction, multiplication, division test

d9380a2

feat. 숫자 입력받기, 입력받은 숫자 띄워주기, AC기능

cd4d723

feat: 사칙연산 기능

0d93b8d

test: cypress value->have.text

b4127cf

test: 테스트 케이스 추가(소수점, AC)

37178bb

fix: 나눗셈 소숫점 버림

5a2f0f9

fix: 첫 숫자가 음수가 들어올 때 오루

3fa286e

refactor: 객체 대신 parsedInput 클래스 활용

19f9bbe

refactor: 객체 대신 parsedInput 클래스 활용

74cc2b2

refactor: total 에 data-type 속성 추가

399280b

입력받을 때 초기화 여부판단 기준으로 삼음

refactor: calculate() if to switch

86f2127

feat. 3자리 제한 기능 checkInputValidation()

7cc044f

fix: = 눌렀을 때 보여주는 문제

9f06aea

fix: 연산자 연속 입력 불가

cf985c7

fix: 처음에 - 입력했을 때 0-로 나타나는 문제

8796702

chore. update tsconfig.json

6b94838

fix: strict syantax applied

fa2a583

hochan222 approved these changes Jun 4, 2021

View reviewed changes

yhshin0 reviewed Jun 4, 2021

View reviewed changes

jwon42 approved these changes Jun 4, 2021

View reviewed changes

	constructor(operator: string, num1: string, num2: string) {
	constructor(operator: string, num1: string, num2: string):void {

		const total = document.getElementById("total") as HTMLElement;
		let prompt = total.innerHTML as string;

	const prompt= document.getElementById("total") as HTMLElement;
	const prompt = document.getElementById("total") as HTMLElement;

[yechoi] typescript-calculator #1

Are you sure you want to change the base?

[yechoi] typescript-calculator #1

Uh oh!

Conversation

yechoi42 commented May 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hochan222 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!

hochan222 Jun 4, 2021 • 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!

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!

jwon42 Jun 4, 2021 • 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yechoi42 commented May 31, 2021 •

edited

Loading

hochan222 Jun 4, 2021 •

edited

Loading

jwon42 Jun 4, 2021 •

edited

Loading