Skip to content

step 2-2 feat: 출력함수구현, refactor: switch문 객체로 수정#88

Merged
honux77 merged 2 commits intocode-squad:wo0kgodfrom
pocokim:step2-1
Apr 12, 2019
Merged

step 2-2 feat: 출력함수구현, refactor: switch문 객체로 수정#88
honux77 merged 2 commits intocode-squad:wo0kgodfrom
pocokim:step2-1

Conversation

@pocokim
Copy link
Copy Markdown

@pocokim pocokim commented Apr 9, 2019

app.js - 실행 파일
calc.js - 계산 관련 함수
check.js - validate 관련 함수

  1. 위와 같이 모듈화를 하였습니다.
  2. getArea함수를 구현하였습니다.
  • getRecursiveCircle 함수로 1부터 n까지 넓이를 계산하는 함수를 만들었는데
    이렇게 따로 함수를 만드는게 좋을지, circleArea 함수안에 재귀적으로 넓이를 계산하는 함수를 만드는게 좋을지 여쭤보고싶습니다!

app.js - 실행 파일
calc.js - 계산 관련 함수
check.js - validate 관련 함수
@pocokim pocokim changed the title feat: add polygon-calculator step 2-1 feat: add polygon-calculator Apr 9, 2019
@crongro
Copy link
Copy Markdown
Contributor

crongro commented Apr 9, 2019

  • getRecursiveCircle 함수로 1부터 n까지 넓이를 계산하는 함수를 만들었는데
    이렇게 따로 함수를 만드는게 좋을지, circleArea 함수안에 재귀적으로 넓이를 계산하는 함수를 만드는게 좋을지 여쭤보고싶습니다!

글쌔요,
이거다 저거다 단정짓긴 어렵지만, 재귀를 써서 더 복잡한 코드가 된다면 피하는쪽으로 하는 게 좋죠.
그런점에서 getRecursiveCircle 함수로 분리해서 만든 건 좋아보입니다.

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.

코드 깔끔합니다.

그런데 printExecution.. 어쩌구 함수 만드는 미션을 빼먹으셨어요.
그거랑 리뷰드린 거랑 한번 수정해보세요!

}

rectancleArea = function(...param){
const [width , height] = check.checkParam(2,param);
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.

es2015를 이미 잘 활용하시네요.
🥇

calc.js Outdated
}

getArea = function(name,...param){
switch(name){
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.

음 코드가 너무 좋은데요.

음 switch문이나 없애볼까요?
(힌트 객체활용)

@crongro
Copy link
Copy Markdown
Contributor

crongro commented Apr 10, 2019

다음엔,
PR날릴때 페어명을 같이 보내주세요.

sequence 변수를 만들고 getArea에서 push를 하고 나중에 출력하도록 하였습니다. 이때 sequence 변수는 객체의 속성으로 정의하였습니다.

switch문 대신에 polygonAreaFunction 객체를 만들고

getArea에서 입력받은 문장을 key값으로 , value는 함수를 연결하였습니다.

출력해주는 printExecutionSequence 함수에서 값의 변경을 용이하게 하기 위해 객체로 값을 넘겨주었습니다.
@pocokim
Copy link
Copy Markdown
Author

pocokim commented Apr 11, 2019

1.1 printExecutionSequence 함수 구현하기

sequence 변수를 만들고 getArea에서 push를 하고 나중에 출력하도록 하였습니다. 이때 sequence 변수는 객체의 속성으로 정의하였습니다.

1.2 switch문 대신 객체로 변경하기

switch문 대신에 polygonAreaFunction 객체를 만들고

getArea에서 입력받은 문장을 key값으로 , value는 함수를 연결하였습니다.

1.3 sequence에 객체로 인자를 넘김

출력해주는 printExecutionSequence 함수에서 값의 변경을 용이하게 하기 위해 객체로 값을 넘겨주었습니다.

궁금한점

  1. circle을 입력받았을때 param의 길이가 1일때 , 2일때 객체안에서 처리되도록 하고싶은데 어떻게 해야할지 몰라서 getArea 안에서 if-else문을 작성하였습니다.
    if(name === 'circle' && param.length === 2) {
        answer = getRecursiveCircle(...param);
    }
    else {answer =  polygonAreaFunction[name](...param);}

polygonAreaFunction 안에서 처리할 수 있는 방법이 없을까요?

  1. polygonAreaFunction 대신 this를 사용하고 싶은데 this를 사용하니 module.exports가 this를 가리키는것을 알 수있었습니다.

    calc-this.js는 this를 이용해 구현한 코드입니다. module.exports는 밖으로 내보낼 부분을 정의하는 객체이기때문에 의미에 맞는것같지 않지만 this를 이용하기 위해 위와같은 방식으로 구현하였습니다. 저희 코드에서 this를 사용하려면 어떻게 해야할까요?

@pocokim
Copy link
Copy Markdown
Author

pocokim commented Apr 11, 2019

step 2-2 로 올렸어야했는데 처음 pr을 step 2-1 브랜치로 올렸네요.
pair 은 wook 과 sel-dev 입니다!

@pocokim pocokim changed the title step 2-1 feat: add polygon-calculator step 2-2 feat: 출력함수구현, refactor: switch문 객체로 수정 Apr 11, 2019
}
}

module.exports = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getArea 외에는 export할 필요가 없지 않을까요?

Copy link
Copy Markdown

@honux77 honux77 left a comment

Choose a reason for hiding this comment

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

수고하셨어요. 👍

@honux77 honux77 merged commit 9807fb7 into code-squad:wo0kgod Apr 12, 2019
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.

3 participants