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

[수연] #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[수연] #8

wants to merge 1 commit into from

Conversation

SooY2
Copy link

@SooY2 SooY2 commented Nov 11, 2023

  • 제목은 [ 이름 ] 으로 작성해주세요!

🌮 무엇을: switch case 문으로 각 계산을 나누어 계산기를 구현했습니다!

  • 무엇을 개발했는지 짧게 설명해주세요! (개발 완료된 부분까지 설명)

🌮 어떻게:

  • 어떻게 개발을 했는지 구체적으로 적어주세요!
  •   case 'sum':
        result = preNum+postNum;
        break;
      case 'sub':
        result = preNum-postNum;
        break;
      case 'mul':
        result = preNum*postNum;
        break;
      case 'divide':
        result = preNum/postNum;
        break;
      default:
        result="";
        break;
    }```
    
    

🌮 얼마나:

  • 얼마나 걸렸나요? 2h
  • 어떤 부분이 어려웠나요? 구현하는거 자체는 어렵지 않았지만 타스를 써보면서 이게 뭔지 검색하면서 알아보는게 오래걸렸어요!!

🌮 구현 사항:

  • 필수 구현 사항이 완료된 부분의 영상을 첨부해주세요
2023-11-12.00.56.15.mov

@hoeun0723 hoeun0723 linked an issue Nov 13, 2023 that may be closed by this pull request
Copy link
Contributor

@hoeun0723 hoeun0723 left a comment

Choose a reason for hiding this comment

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

궁금한 점 몇가지와, 더 좋은 코드가 무엇일까!? 에 대하여 고민한 부분을 남깁니다:)

좋은 코드 감사해요!!

Copy link
Contributor

Choose a reason for hiding this comment

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

오옷!! 혹시 이 파일은 미리 js로 작성해보다가 남겨진 파일인가요 ~??

현재 html에서 js 파일을 로드 하고 있어서 ts로 짠 코드가 작동이 안 되는 거 같아요 ㅠ.ㅠ

Copy link
Member

Choose a reason for hiding this comment

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

앗 혹시 컴파일한건가요?? 이거 vite라 그냥 했어도 됐는데!

Comment on lines +16 to +32
switch (cal.value) {
case 'sum':
result = preNum+postNum;
break;
case 'sub':
result = preNum-postNum;
break;
case 'mul':
result = preNum*postNum;
break;
case 'divide':
result = preNum/postNum;
break;
default:
result="";
break;
}
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) => a / b,
};

if (cal.value in operations) {
  result = operations[cal.value](preNum, postNum);
} else {
  result = "올바른 연산을 선택하세요.";
}

이런식으로요!
코드가 더 간결해지고, 확장 가능성도 높일 수 있을 것이라 생각했어요 !

Copy link
Member

Choose a reason for hiding this comment

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

옴마나,,,, 진ㅉㅏ 갓갓!! 대신에 타스니까 또 여기에 타입 넣는 거를 잊지 않아야 해용!
`const operations = {
sum: (a: number, b: number): number => a + b,
sub: (a: number, b: number): number => a - b,
mul: (a: number, b: number): number => a * b,
divide: (a: number, b: number): number => a / b,
};

let result: number | string;

if (cal.value in operations) {
result = operations[cal.value](preNum, postNum);
} else {
result = "올바른 연산을 선택하세요.";
}
`

Copy link
Member

@hae2ni hae2ni left a comment

Choose a reason for hiding this comment

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

굿굿! 넘 수고했어요! 완전짱이야! 👍

Copy link
Member

Choose a reason for hiding this comment

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

앗 혹시 컴파일한건가요?? 이거 vite라 그냥 했어도 됐는데!


const $ = (selector:string) => document.querySelector(selector) as HTMLElement;

const num1 = $('#num1') as HTMLInputElement;;
Copy link
Member

Choose a reason for hiding this comment

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

먁 오타생긴 거 같아ㅠㅠ 에러 뜰 거 같은ㄷㅔ!

const num2 = $('#num2') as HTMLInputElement;;
const cal=$('select[name="calculate"]') as HTMLSelectElement;
const resultTag=$('h3') as HTMLElement;
const btn = $('button') as HTMLButtonElement;;
Copy link
Member

Choose a reason for hiding this comment

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

요기도!!

Comment on lines +16 to +32
switch (cal.value) {
case 'sum':
result = preNum+postNum;
break;
case 'sub':
result = preNum-postNum;
break;
case 'mul':
result = preNum*postNum;
break;
case 'divide':
result = preNum/postNum;
break;
default:
result="";
break;
}
Copy link
Member

Choose a reason for hiding this comment

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

옴마나,,,, 진ㅉㅏ 갓갓!! 대신에 타스니까 또 여기에 타입 넣는 거를 잊지 않아야 해용!
`const operations = {
sum: (a: number, b: number): number => a + b,
sub: (a: number, b: number): number => a - b,
mul: (a: number, b: number): number => a * b,
divide: (a: number, b: number): number => a / b,
};

let result: number | string;

if (cal.value in operations) {
result = operations[cal.value](preNum, postNum);
} else {
result = "올바른 연산을 선택하세요.";
}
`

result = preNum*postNum;
break;
case 'divide':
result = preNum/postNum;
Copy link
Member

Choose a reason for hiding this comment

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

postNum이 0인 예외으 경우도 빼주면 좋을 것 같아요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

계산기 프로젝트 구현 내용
3 participants