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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ <h1>혜인이의 계산기</h1>
</header>
<main>
<section>
<input type="number" />
<select name="calculate">
<option value="sum">+</option>
<option value="sub">-</option>
<option value="mul">x</option>
<option value="divide">/</option>
<input id="first_number" type="text" />
<select id="selector" name="calculate">
<option id="sum" value="sum">+</option>
<option id="sub" value="sub">-</option>
<option id="mul" value="mul">x</option>
<option id="divide" value="divide">/</option>
</select>
<input type="number" />
<input id="second_number" type="text" />
<h2>=</h2>
<h3>정답</h3>
<h3 id="result">정답</h3>
</section>
<button>결과는?</button>
<button type="button" id="result_button">결과는?</button>
</main>
<script type="module" src="/src/main.ts"></script>
</body>
Expand Down
57 changes: 57 additions & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
const $ = (selector: string) =>
document.getElementById(selector) as HTMLElement;
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

DOM 요소를 가져오는데, ID가 존재하지 않아 nul을 반환할 수 있으니, null 체크와 에러처리를 추가해주는 것은 어떤가요 ??

const $ = (selector: string) => {
  const element = document.getElementById(selector);
  if (!element) {
    throw new Error(`Element with ID '${selector}' not found.`);
  }
  return element as HTMLElement;
};

Choose a reason for hiding this comment

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

저는 null 관련 지정을 안 했더니 에러가 무진장 나서 냅다 내 코드에서 null은 발생할 수 없어. 라고 !통해서 지정해두고 넘어갔는데 .... 혠니 코드에서는 따로 null관련 코드가 없는데도 에러 발생 없이 잘 작동했나보군요 ! 신기하네용 .. 뭔가 접근 방식에 따라 null에러가 발생하는 건가 ..?

Choose a reason for hiding this comment

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

저만 그랬던게 아니군요...허허 저는 이번주에 배운 | null 을 붙여서 해결했는데, 혜인언니 코드에는 없어서 어떻게 해결했는지 궁금합니당


const firstNumber = $("first_number") as HTMLInputElement;
const secondNumber = $("second_number") as HTMLInputElement;
const resultBtn = $("result_button") as HTMLButtonElement;
const selector = $("selector") as HTMLSelectElement;
const resultHere = $("result") as HTMLHeadingElement;

function blockString(inputDom: HTMLInputElement): void {
inputDom.addEventListener("input", (e) => {
const input = e.target as HTMLInputElement;
const changeToNum = input.value.replace(/[^0-9.]/g, "");
Copy link

Choose a reason for hiding this comment

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

오 이렇게 하면 잘못 해서 문자가 입력돼도 그냥 생략되는건가용?
ex) 12.a23 => 12.23 이런식으로??

Copy link

Choose a reason for hiding this comment

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

숫자만 입력할 수 있게 숫자를 제외하고""로 바꿔주는거 좋네용!!!

if (input.value != changeToNum) {
alert("숫자입력하세요");
input.value = "";
Comment on lines +15 to +16

Choose a reason for hiding this comment

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

이런 섬세한 에러처리 넘 좋네용

}
});
}

blockString(firstNumber);
blockString(secondNumber);

function calculator() {
const num1 = parseFloat(firstNumber?.value);
const num2 = parseFloat(secondNumber?.value);
const selectorOptions = selector.value;

let result: number | string;

switch (selectorOptions) {
case "sum":
result = num1 + num2;
break;
case "sub":
Comment on lines +30 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

시작부분에 입력이 유효한지 확인하고 오류 처리를 추가해주는 것은 어떤가요 ??

if (isNaN(num1) || isNaN(num2)) {
    result = "숫자를 입력하세요";
  } else {

이런식으로요 !!

하지만 필수적인 요소는 아니라고 생각합니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

그러네그러네!! 게으른 나,,

result = num1 - num2;
break;
case "mul":
result = num1 * num2;
break;
case "divide":
if (num2 !== 0) {
Copy link

Choose a reason for hiding this comment

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

헉 난 완전히 놓친부분... 섬세하다✨

result = num1 / num2;
} else {
result = "계산 불가";
}
Comment on lines +41 to +46

Choose a reason for hiding this comment

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

섬세한 예외처리..너무 좋네요👍

break;
default:
result = "";
break;
}
Comment on lines +31 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

코드 리뷰를 하다가 생각한건데, switch문이 아닌, 객체 매핑 방식으로 변경해보는건 어떤가요 ~??

const operations = {
  sum: (a, b) => a + b,
  sub: (a, b) => a - b,
  mul: (a, b) => a * b,
  divide: (a, b) => (b !== 0 ? a / b : "계산 불가"),
};

if (selectorOptions in operations) {
  result = operations[selectorOptions](num1, num2);
} else {
  result = "";
}

이런식으로요!

코드가 더 간결해지고 확장 가능성이 높아질 것이라고 생각했습니다 !!

Choose a reason for hiding this comment

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

그그 그렇다면 호은이 코드 리뷰를 적용해보려면 객체를 interface로 만들게 되는 것인가요 ?
혹은 interface로 key-value 타입 지정을 해두고,
const operations의 타입에 만들어둔 interface를 대입하면 되는 것인가 ..?

interface Operations {
  [key: string]: (a: number, b: number) => number | string;
}

const operations : Operations {
 ...
}

이렇게 ..? 🥹

resultHere.textContent = result.toString();
firstNumber.value = "";
secondNumber.value = "";
}

resultBtn.addEventListener("click", calculator);