Conversation
* 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
…rintExecutionSequence
crongro
left a comment
There was a problem hiding this comment.
대부분 구현 다 잘 했네요~
자잘한 수정좀더 해보세요!
polygonSize.js
Outdated
| @@ -0,0 +1,80 @@ | |||
| var arr = []; | |||
There was a problem hiding this comment.
변수명도 무심하게 arr말고 제대로 의미를 부여해줘보세요.
polygonSize.js
Outdated
| } | ||
| console.log('원 모두의 합은 ' + sum + '입니다.'); | ||
| return; | ||
| } else if (arguments.length != 1) { |
There was a problem hiding this comment.
else if 조건 두개를 먼저 체크해서 return 시키고,
위의 코드를 구현하면 위쪽 코드의 indent가 없어지겠네요.
polygonSize.js
Outdated
| function sizeSquare(lowerSide, height) { | ||
| if (arguments.length != 2) { | ||
| return console.log('인자의 개수가 맞지 않습니다.'); | ||
| } else if (isNum(lowerSide) || isNum(height)) { |
There was a problem hiding this comment.
참고로,
isNum이 배열이나 숫자를 받을 수 있게 똑똑하게 동작한다면, 여기코드는 좀더 보기 좋게 될 듯 하네요.
arguments넘겨주면 되니까요.
polygonSize.js
Outdated
| } | ||
|
|
||
| function printExecutionSequence() { | ||
| arr.forEach(function(v, i) { |
There was a problem hiding this comment.
다음엔 요런경우에 arrow function써보기.
…ay, and Revise PrintExecutionSequence function(revise forEach method to use arrow function),
…on for isNum function
…se inner contents
|
안보이던 짜잘한 버그들이 푸쉬하자마자 보이기 시작해서...혹시 보시지 않으셨다면 수정해서 다시 푸쉬해도 될까요? |
…rCase to lowerCase
|
수정하여 올렸습니다! 감사합니다 ㅎㅎ |
crongro
left a comment
There was a problem hiding this comment.
잘 구현했습니다. 👍
이번렉쳐에서 배울 부분은 여기서 마무리 해도 될 것 같네요.
몇개 리뷰는 참고로 남겨드리니 살펴보시고요.
| @@ -1,9 +1,9 @@ | |||
| //변수 const나 let으로 변경, 변수이름 개선 | |||
| const ExecutionSequenceArray = []; | |||
| const executionSequenceArray = []; | |||
There was a problem hiding this comment.
참고로, 배열변수는 Array라고 명확하게 쓰는 경우도 있지만, list나 s(복수형)으로 하는 경우가 좀더 일반적인거 같네요.
자바스크립트는 타입을 변수에 표현하는게 참 어렵긴합니다.
|
|
||
| //1. isNum함수 개선 isNum함수를 위한 isThatNum 함수만들기 | ||
| function isNum(number1, number2, number3) { | ||
| for (let i = 1; i <= arguments.length; i++) { |
There was a problem hiding this comment.
for 문은 i++을 내가 해줘야 하는등 좀 불편한면이 있어요.
배열을 순차적으로 돌아야 하는 경우면 for-of를 사용하는 것이 이제 표준입니다.
앞으로는 for-of로 대체할 수 있을때는 for-of를 사용해보세요~
| //3. 계산하는 함수들의 이름과 인자들의 명칭들 모두 변경 | ||
| function calcCircleWidth(radius1, radius2) { | ||
| if (radius1 > radius2) { | ||
| console.log(radius1 + '부터' + radius2 + '까지라구요? 다시 한번 생각해주세요') |
| } | ||
|
|
||
| function calcSylinderWidth (sylinderRadius, sylinderHeight) { | ||
| if(isNum(...arguments) && arguments.length === 2) { |
There was a problem hiding this comment.
넓이를 계산하는 함수마다 이부분이 중복이죠?
isNum(...arguments) && arguments.length === 2
이런부분을 함수를 사용해서 중복을 제거할 수 있습니다.
* 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.
|
@HTMLhead PR보내실때 master branch 로 보내지 않도록 잘 봐주세요 ~ ! |
알려주신 피드백을 통해서 수정하였습니다. getArea함수를 추가하였고, printExecutionSequence함수를 추가하였습니다. sizeCircle함수에 원 모두의 합을 구하는 방법을 추가하였습니다.