Skip to content

1차 다각형넓이구하기#32

Merged
crongro merged 2 commits intocode-squad:masterfrom
alisonjh:step001
Sep 14, 2018
Merged

1차 다각형넓이구하기#32
crongro merged 2 commits intocode-squad:masterfrom
alisonjh:step001

Conversation

@alisonjh
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

수고하셨어요!
낯선부분을 시도하고 적용하신 거 같아요.
잘하셨어요.

몇개 리뷰도 남겼으니 참고해서 수정해보세요
그리고 질문도 남겼습니다.
질문에 대답도 좀 해보세요~

step1.js Outdated

//1. 원의 넓이 계산
function circleArea (radius) {
if ([radius].length != arguments.length) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 비교가 의미하는 게 어떤거죠?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

인수의 개수 확인을 설계를 위한 문장이었습니다.

각 함수 안에서 let requireAugs = [radius];
와 같이 필수 인수를 array에 저장하고 값을 비교하는 방법이 나을까요?

하드코딩으로 변수를 저장해야되는 부분과 그렇지 않은 부분을 구분하는 게 어렵네요.
하드 코딩을 사용하지 않고,
함수를 설정할 때의 parameter의 length와 arguments의 length를 구분할 수는 없을까요?
인수의 개수를 확인 코드를 설계할 때 가장 궁금했던 점이어서 답변에도 덧붙여 봅니다.

step1.js Outdated
//다각형의 넓이 구하기

//1. 원의 넓이 계산
function circleArea (radius) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

함수는 이름으로 무엇을 하는지 드러내야 합니다. 그래야 코드를 읽기 좋은데요.
주로 동사+명사로 표현해요.
그렇게 이름을 변경해보실래요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

네 함수 이름 변경해서 push하겠습니다! :)

step1.js Outdated
}

if (!verification(radius)) {
let pi = 3.14;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let보다는 const가 더 잘 어울립니다! 왜그럴까요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pi는 절대 변하지 않을 값이니 상수인 const를 써야되네요! 이 부분도 수정하겠습니다 :)

step1.js Outdated

if (!verification(width,height)) {
let result = Number(width) * Number(height);
return console.log(result);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

squareArea가 console.log의결과를 반환하고 있는데요. 의미가 없는 코드입니다.
console.log 의 반환값은 무엇인지 확인해보세요.

step1.js Outdated

//5. 숫자 확인 함수
function verification (val) {
if ( isNaN(val) === true ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NaN이 의미하는게 뭔지 이해하고 계세요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NaN는 Not a Number의 약자이며, isNaN() 메소드를 썼을 때 true값이 나오면 그 인자는 숫자가 아닌 그 여집합이다, 이렇게 이해하고 있었습니다. MDN에 가서 한 번 더 읽어보고 올게요!

Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

수정사항 모두 확인했어요. 잘 하셨네요.
여기까지 승인/머지 합니다.
코드리뷰 가이드 다시보시면서, 이제 rebase받아서 다음 step브랜치 만들고 해보세요. ^^

아 그리고
브랜치이름은 step001 말고 step1, step2로 해주시고요~

@crongro crongro merged commit 095706e into code-squad:master Sep 14, 2018
crongro added a commit that referenced this pull request Sep 14, 2018
* 1차 다각형넓이구하기 (#32)

* 1차 다각형넓이구하기

* 1.함수명 변경 2.숫자판별을 함수가 아닌 개별 함수내의 조건으로 재설정 3.let pi > const pi 4.return 오류 수정

* Step2 (#34)

* 다각형 넓이 (#29)

* index of polygonSize

* Full index of html and get area of circle, square, trapezoid, cylinder

* restart

* add sizeCircle function

* add function sizeSquare, add testcode

* add function sizeTrapezoid, add test code

* add function sizeSylinder, add distinguish test code, add test code

* modify by referring to feedback, add function getArea, add function printExecutionSequence

* Revise var to const, Change variable name arr -> ExecutionSequenceArray, and Revise PrintExecutionSequence function(revise forEach method to use arrow function),

* revise isNum function to use 'for' statement and add typenumberfunction for isNum function

* modifiy function sizeCircle to CalcCircleWidth and change inner contents

* Modify sizeSquare function to CalcRectWidth and change inner contents

* Modify sizeTrapezoid function to calcTrapezoidWidth function and revise inner contents

* Modify sizeSylinder functino to calcSylinderWidth function and revise inner context

* Modify getArea function to check Number of arguments

* Revise little bugs,(add return to calcSylinderWidth, change some upperCase to lowerCase

* Revert "Step2 (#34)"

This reverts commit 97ff01e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants